Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework ElectrumClient #512

Merged
merged 16 commits into from
Sep 11, 2023
Merged

Rework ElectrumClient #512

merged 16 commits into from
Sep 11, 2023

Commits on Jul 11, 2023

  1. Configuration menu
    Copy the full SHA
    813b520 View commit details
    Browse the repository at this point in the history
  2. Move socketBuilder to connect()

    This is only used when connecting, it doesn't need to be an argument of
    the `ElectrumClient`.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    94f2b90 View commit details
    Browse the repository at this point in the history
  3. Refactor and document linesFlow

    TCP sockets with electrum servers require some non-trivial code to
    reconstruct individual messages from the bytes received on the socket.
    
    We process bytes in chunks, then reconstruct utf8 strings, and finally
    split those strings at newline characters into individual messages.
    
    We document that code, rename fields to make it easier to understand,
    and add unit tests. We remove it from the TCP socket abstraction, since
    this is specific to electrum connections.
    
    We also change the behavior in case the socket is closed while we have
    buffered a partial message: it doesn't make sense to emit it, as listeners
    won't be able to decode it.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    214848e View commit details
    Browse the repository at this point in the history
  4. Remove duplicate connectionState flow

    This is a pure 1:1 mapping from `connectionStatus`, there is no reason for
    that duplication. Clients can trivially migrate or `map` using the
    `toConnectionState` helper function.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    e66b313 View commit details
    Browse the repository at this point in the history
  5. Rework connection flow

    We clean up the coroutine hierarchy: the `ElectrumClient` has a single
    internal coroutine that is launched once the connection has been
    established with the electrum server. This coroutine launches three child
    coroutines and uses supervision to gracefully stop if the connection is
    closed or an error is received.
    
    Connection establishment happens in the context of the job that calls
    `connect()` and doesn't throw exceptions.
    
    We revert the introduction of a `CoroutineExceptionHandler` inside the
    JVM socket code, as it shouldn't be needed anymore.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    eabcdc4 View commit details
    Browse the repository at this point in the history
  6. Add timeout to connection establishment

    Otherwise we may be stuck in the `Connecting` state.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    e914f3e View commit details
    Browse the repository at this point in the history
  7. Remove unnecessary RPC calls exposed

    We just keep `getHeader` / `getHeaders` which can be handy.
    t-bast committed Jul 11, 2023
    Configuration menu
    Copy the full SHA
    0377d0d View commit details
    Browse the repository at this point in the history

Commits on Jul 12, 2023

  1. Handle electrum server errors

    On most RPC calls, we can gracefully handle server errors. Since we cannot
    trust the electrum server anyway, and they may lie to us by omission, this
    doesn't downgrade the security model.
    
    The previous behavior was to throw an exception, which was never properly
    handled and would just result in a crash of the wallet application.
    t-bast committed Jul 12, 2023
    Configuration menu
    Copy the full SHA
    253b853 View commit details
    Browse the repository at this point in the history
  2. Add timeout and retry to RPC calls

    We add an explicit timeout to RPC calls and a retry. If that retry also
    fails, two strategies are available:
    
    - handle the error and gracefully degrade (non-critical RPC calls)
    - disconnect and retry with new electrum server (critical RPCs such as
      subscriptions)
    
    The timeout can be updated by the application, for example when a slow
    network is detected or when Tor is activated.
    t-bast committed Jul 12, 2023
    Configuration menu
    Copy the full SHA
    13b06fa View commit details
    Browse the repository at this point in the history

Commits on Jul 28, 2023

  1. Fix iOS tests

    When connecting to an invalid address in tests on iOS, the error isn't
    reported by the native socket which just hangs until the test times out.
    
    We lower the timeout on the `connect` call, which previously had the same
    value as the test timeout. We thus fail the connection attempt after
    1 second and are able to reconnect to a different server.
    
    The very low RPC timeout was flaky on iOS: because the clock used depends
    on the actual dispatcher, it sometimes triggered only after receiving a
    valid response from the server, which made the test fail. Setting it to
    0ms ensures that the is immediately cancelled with a timeout exception
    regardless of the dispatcher used.
    t-bast committed Jul 28, 2023
    Configuration menu
    Copy the full SHA
    d60e6e7 View commit details
    Browse the repository at this point in the history
  2. Add CoroutineExceptionHandler on tls sockets

    We actually still need it until ktorio/ktor#3690
    is integrated into a ktor release.
    t-bast committed Jul 28, 2023
    Configuration menu
    Copy the full SHA
    2b70f09 View commit details
    Browse the repository at this point in the history

Commits on Aug 8, 2023

  1. Remove remaining test delay

    This was forgotten after e2e tests.
    t-bast committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    d691a0b View commit details
    Browse the repository at this point in the history
  2. Close TCP socket when race with timeout

    Since `withTimeout` runs concurrently, it may throw the timeout exception
    after we've established the TCP connection, so we need to release it
    if we can (if we have a pointer to it).
    t-bast committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    4502d86 View commit details
    Browse the repository at this point in the history

Commits on Aug 11, 2023

  1. Configuration menu
    Copy the full SHA
    b2f72d3 View commit details
    Browse the repository at this point in the history
  2. s/block/suspend

    t-bast committed Aug 11, 2023
    Configuration menu
    Copy the full SHA
    ad86714 View commit details
    Browse the repository at this point in the history

Commits on Sep 6, 2023

  1. Remove currentBlockHeight from extensions methods

    We can actually get it from the connection status, which simplifies the
    way callers use those functions.
    t-bast committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    75ab436 View commit details
    Browse the repository at this point in the history