Support for confirmations when listening to events

[peterbroadhurst]

• One of the key work items for support of public blockchains per Support for public blockchains #149
• Adds a new BlockConfirmationManager (BCM) component
  • Enabled through a new confirmations configuration sub-section section in openapi

  rest:
    rest-gateway:
      rpc:
        url: https://rinkeby.infura.io/v3/...
      openapi:
        catchupModeBlockGap: 50
        catchupModePageSize: 50
        confirmations:
          enabled: true               # Turn on the feature
          requiredConfirmations: 20   # The required number of confirmations to deliver the event
          includeInPayload: true      # Includes an array of the block confirmations in the event
        decimalTransactionIndex: true # Gives consistency in decimal formatting of transactionIndex
      maxTXWaitTime: 300
      maxInFlight: 100

  • Sits in-between the LogProcessor and the EventStream
  • Responsible for detecting confirmations, and only propagating events once confirmed
  • Events stay in-flight until confirmed, so checkpointing logic functions unchanged
  • Performs polling based block listening to add confirmations as soon as they arrive
  • Handles confirmations of historic events when replaying
  • Handles forks that invalidate at any point in the confirmation list
  • Block caching avoids excessive JSON/RPC calls against the node
  • Tested with the Infura gateway for the Rinkeby network
• Updates handling of removed events
  • Processed by BCM when enabled
  • Ignored otherwise
• Adds migration for blockedReryDelaySec config typo to blockedRetryDelaySec
• Adds a hexFormatTransactionIndex option that can be set to false to disable the current behavior where transactionIndex is delivered as 0x01 etc. whereas blockNumber/logIndex are delivered as the string "1"
  • The default has not been changed to avoid impact on production apps

[codecov-commenter]

Codecov Report

Merging #207 (2faf053) into main (dc6f8a7) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
+ Coverage   97.24%   97.30%   +0.05%     
==========================================
  Files          58       59       +1     
  Lines        6944     7246     +302     
==========================================
+ Hits         6753     7051     +298     
- Misses        146      148       +2     
- Partials       45       47       +2     

Impacted Files	Coverage Δ
ethconnect/internal/events/logprocessor.go	98.31% <0.00%> (-1.69%)	⬇️
ethconnect/internal/events/submanager.go	99.27% <0.00%> (-0.73%)	⬇️
ethconnect/internal/tx/txnprocessor.go	100.00% <0.00%> (ø)
ethconnect/internal/events/block_confirmations.go	100.00% <0.00%> (ø)
ethconnect/internal/events/eventstream.go	96.99% <0.00%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc6f8a7...2faf053. Read the comment docs.

[vdamle]

👍🏼

[vdamle]

should we enforce a safe upper bound (no idea what that should be though!)?

[peterbroadhurst]

I guess we haven't elsewhere on these. I propose no is ok for now

[vdamle]

Not requesting as part of this PR given I'm providing this comment at the last minute, it would be good to have a follow on that wraps updates to bcm.pending with a RWMutex - I'm assuming there's a high likelihood of concurrent updates.

[peterbroadhurst]

Agree a good candidate to look at in the future, and deferrable.

[vdamle]

Wouldn't we have already removed the key from bcm.pending since we moved this event from bcm.pending to confirmed?

[peterbroadhurst]

Agreed re-reading the code