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.

Sign up for GitHub

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

Jump to bottom

[2/3]: implement blockbeat #9

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

[2/3]: implement blockbeat #9

wants to merge 42 commits into from

Conversation

yyforyongyu
Copy link
Owner

@yyforyongyu yyforyongyu commented Jun 17, 2024 •
edited
Loading

This PR introduces a minimal version of the new service Blockbeat as described in lightningnetwork#7952, to handle block syncronization among different subsystems.

During startup, blockbeat consumers are registered in the BlockbeatDispatcher, a subservice that's responsible for dispatching new blockbeats to its consumers and waiting for its consumers to finish processing the blocks. If any of the consumers failed to process the block under 30s, or encountered an error during block processing, the system will shut down as it's critical to handle blocks.

This PR focuses on implementing blockbeat Consumer interface for ChainArb, UtxoSweeper and TxPublisher, the following PR focuses on finalizing blockbeat processing in ChainArb's subservices - ChannelArbitrator, chainWatcher, and ContractResolver.

Overview

The flow of the blockbeat process is shown below, whenever a new block is arrived, it goes through the waterfall like this,

  1. blockbeat dispatcher receives a new block epoch and makes a blockbeat, and sends it to its consumers sequentially.
  2. ChainArb receives the blockbeat, processes it and signals back when done.
  3. UtxoSweeper receives the blockbeat, processes it and signals back when done.
  4. TxPublisher receives the blockbeat, processes it and signals back when done.
  5. This new block is now considered processed by the blockbeat dispatcher.
sequenceDiagram
		autonumber
		participant bb as BlockBeat
		participant cc as ChainArb
		participant us as UtxoSweeper
		participant tp as TxPublisher
		
		note left of bb: 0. received block x,<br>dispatching...
		
    note over bb,cc: 1. send block x to ChainArb,<br>wait for its done signal
		bb->>cc: block x
		rect rgba(165, 0, 85, 0.8)
      critical signal processed
        cc->>bb: processed block
      option Process error or timeout
        bb->>bb: error and exit
      end
    end

    note over bb,us: 2. send block x to UtxoSweeper, wait for its done signal
		bb->>us: block x
		rect rgba(165, 0, 85, 0.8)
      critical signal processed
        us->>bb: processed block
      option Process error or timeout
        bb->>bb: error and exit
      end
    end

    note over bb,tp: 3. send block x to TxPublisher, wait for its done signal
		bb->>tp: block x
		rect rgba(165, 0, 85, 0.8)
      critical signal processed
        tp->>bb: processed block
      option Process error or timeout
        bb->>bb: error and exit
      end
    end
Loading

NOTE: itests are failing in this PR and is fixed in the final PR.

TODO

  • add unit tests for new code
  • add readme

@github-actions GitHub Actions
Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from 8242ffd to bc13d54 Compare June 20, 2024 13:43
@yyforyongyu yyforyongyu force-pushed the yy-blockbeat branch 3 times, most recently from 3592aa1 to 7df2d94 Compare June 21, 2024 10:05
@coveralls
Copy link

coveralls commented Jun 21, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9611908009

Details

  • 125 of 385 (32.47%) changed or added relevant lines in 19 files are covered.
  • 17 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.06%) to 57.317%
Changes Missing Coverage Covered Lines Changed/Added Lines %
log.go 0 1 0.0%
chainio/log.go 6 9 66.67%
contractcourt/channel_arbitrator.go 42 46 91.3%
sweep/fee_bumper.go 9 15 60.0%
sweep/sweeper.go 13 22 59.09%
chainio/interface.go 7 21 33.33%
contractcourt/chain_arbitrator.go 11 35 31.43%
server.go 0 35 0.0%
chainio/blockbeat.go 15 76 19.74%
chainio/dispatcher.go 0 103 0.0%
Files with Coverage Reduction New Missed Lines %
contractcourt/channel_arbitrator.go 1 75.37%
server.go 1 0.31%
lnwallet/test/test_interface.go 1 77.73%
queue/gc_queue.go 2 96.51%
sweep/sweeper.go 2 44.36%
sweep/fee_bumper.go 2 72.98%
watchtower/wtdb/range_index.go 2 96.38%
funding/manager.go 6 68.84%
Totals Coverage Status
Change from base Build 9598606296: -0.06%
Covered Lines: 92581
Relevant Lines: 161524
💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from bc13d54 to 01b173c Compare June 24, 2024 13:59
@coveralls
Copy link

coveralls commented Jun 24, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9646782258

Details

  • 125 of 377 (33.16%) changed or added relevant lines in 19 files are covered.
  • 51 unchanged lines in 14 files lost coverage.
  • Overall coverage decreased (-0.07%) to 57.329%
Changes Missing Coverage Covered Lines Changed/Added Lines %
log.go 0 1 0.0%
chainio/log.go 6 9 66.67%
contractcourt/channel_arbitrator.go 42 46 91.3%
sweep/fee_bumper.go 9 15 60.0%
sweep/sweeper.go 13 22 59.09%
chainio/interface.go 7 21 33.33%
contractcourt/chain_arbitrator.go 11 35 31.43%
server.go 0 27 0.0%
chainio/blockbeat.go 15 76 19.74%
chainio/dispatcher.go 0 103 0.0%
Files with Coverage Reduction New Missed Lines %
contractcourt/channel_arbitrator.go 1 75.37%
server.go 1 0.31%
queue/gc_queue.go 2 96.51%
lnwallet/chancloser/chancloser.go 2 81.13%
sweep/sweeper.go 2 44.36%
sweep/fee_bumper.go 2 72.98%
htlcswitch/mailbox.go 2 93.9%
channeldb/channel.go 2 72.98%
build/prefix_log.go 3 72.22%
watchtower/wtdb/migration4/range_index.go 3 50.9%
Totals Coverage Status
Change from base Build 9646711542: -0.07%
Covered Lines: 92602
Relevant Lines: 161526
💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from 01b173c to 9224414 Compare June 25, 2024 13:13
@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from 9224414 to 3b91244 Compare June 27, 2024 01:29
@yyforyongyu yyforyongyu force-pushed the yy-blockbeat branch 2 times, most recently from 6f652ac to e65366e Compare June 27, 2024 13:47
@yyforyongyu yyforyongyu changed the title implement blockbeat [2/3]: implement blockbeat Jun 27, 2024
@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from 3b91244 to fb67045 Compare June 27, 2024 18:23
@coveralls
Copy link

coveralls commented Jun 27, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9702800783

Details

  • 131 of 382 (34.29%) changed or added relevant lines in 19 files are covered.
  • 61 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.07%) to 57.318%
Changes Missing Coverage Covered Lines Changed/Added Lines %
log.go 0 1 0.0%
chainio/log.go 6 9 66.67%
contractcourt/channel_arbitrator.go 42 46 91.3%
sweep/fee_bumper.go 9 17 52.94%
sweep/sweeper.go 13 22 59.09%
chainio/interface.go 7 19 36.84%
contractcourt/chain_arbitrator.go 11 35 31.43%
server.go 0 27 0.0%
chainio/blockbeat.go 21 82 25.61%
chainio/dispatcher.go 0 102 0.0%
Files with Coverage Reduction New Missed Lines %
contractcourt/channel_arbitrator.go 1 75.37%
server.go 1 0.31%
queue/gc_queue.go 2 96.51%
invoices/invoiceregistry.go 2 82.44%
sweep/sweeper.go 2 44.36%
sweep/fee_bumper.go 2 73.06%
htlcswitch/switch.go 2 72.39%
watchtower/wtdb/migration8/range_index.go 2 13.18%
channeldb/migration30/migration.go 3 82.84%
htlcswitch/mailbox.go 4 93.16%
Totals Coverage Status
Change from base Build 9701688598: -0.07%
Covered Lines: 92563
Relevant Lines: 161490
💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the yy-blockbeat branch 3 times, most recently from 040d363 to 651886f Compare June 29, 2024 02:18
@coveralls
Copy link

coveralls commented Jun 29, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9720833654

Details

  • 136 of 441 (30.84%) changed or added relevant lines in 20 files are covered.
  • 42 unchanged lines in 11 files lost coverage.
  • Overall coverage decreased (-0.09%) to 57.305%
Changes Missing Coverage Covered Lines Changed/Added Lines %
log.go 0 1 0.0%
chainio/log.go 6 9 66.67%
contractcourt/channel_arbitrator.go 43 47 91.49%
sweep/fee_bumper.go 9 20 45.0%
chainio/interface.go 10 29 34.48%
sweep/sweeper.go 13 36 36.11%
contractcourt/chain_arbitrator.go 11 35 31.43%
server.go 0 35 0.0%
chainio/blockbeat.go 21 82 25.61%
chainio/dispatcher.go 0 124 0.0%
Files with Coverage Reduction New Missed Lines %
contractcourt/channel_arbitrator.go 1 75.52%
server.go 1 0.31%
invoices/invoiceregistry.go 2 82.26%
sweep/sweeper.go 2 44.58%
sweep/fee_bumper.go 2 72.74%
htlcswitch/mailbox.go 2 93.9%
htlcswitch/switch.go 2 72.39%
watchtower/wtdb/migration8/range_index.go 2 13.18%
channeldb/migration30/migration.go 3 82.84%
htlcswitch/link.go 6 69.65%
Totals Coverage Status
Change from base Build 9701688598: -0.09%
Covered Lines: 92566
Relevant Lines: 161532
💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the yy-blockbeat branch 2 times, most recently from c5d5f0d to d7ffa05 Compare July 1, 2024 06:32
@coveralls
Copy link

coveralls commented Jul 1, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9739338901

Details

  • 136 of 430 (31.63%) changed or added relevant lines in 20 files are covered.
  • 46 unchanged lines in 11 files lost coverage.
  • Overall coverage decreased (-0.07%) to 57.316%
Changes Missing Coverage Covered Lines Changed/Added Lines %
log.go 0 1 0.0%
chainio/log.go 6 9 66.67%
contractcourt/channel_arbitrator.go 43 47 91.49%
sweep/fee_bumper.go 9 20 45.0%
sweep/sweeper.go 13 25 52.0%
chainio/interface.go 10 29 34.48%
contractcourt/chain_arbitrator.go 11 35 31.43%
server.go 0 35 0.0%
chainio/blockbeat.go 21 82 25.61%
chainio/dispatcher.go 0 124 0.0%
Files with Coverage Reduction New Missed Lines %
contractcourt/channel_arbitrator.go 1 75.52%
server.go 1 0.31%
queue/gc_queue.go 2 96.51%
sweep/fee_bumper.go 2 72.74%
htlcswitch/mock.go 2 75.25%
watchtower/wtdb/migration8/range_index.go 2 13.18%
channeldb/migration30/migration.go 3 82.84%
sweep/sweeper.go 4 45.2%
htlcswitch/mailbox.go 4 93.53%
htlcswitch/link.go 6 69.74%
Totals Coverage Status
Change from base Build 9701688598: -0.07%
Covered Lines: 92575
Relevant Lines: 161518
💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from fb67045 to 799f05b Compare July 2, 2024 11:21
@yyforyongyu yyforyongyu force-pushed the yy-prepare-blockbeat branch from 799f05b to a0393a0 Compare July 4, 2024 12:11
yyforyongyu and others added 29 commits November 21, 2024 20:46
@yyforyongyu
sweep: add new interface method Immediate
fb4435e 
This prepares the following commit where we now let the fee bumpr
decides whether to broadcast immediately or not.
@yyforyongyu
sweep: handle inputs locally instead of relying on the tx
a7fd0c5 
This commit changes how inputs are handled upon receiving a bump result.
Previously the inputs are taken from the `BumpResult.Tx`, which is now
instead being handled locally as we will remember the input set when
sending the bump request, and handle this input set when a result is
received.
@yyforyongyu
sweep: add handleInitialBroadcast to handle initial broadcast
93758fe 
This commit adds a new method `handleInitialBroadcast` to handle the
initial broadcast. Previously we'd broadcast immediately inside
`Broadcast`, which soon will not work after the `blockbeat` is
implemented as the action to publish is now always triggered by a new
block. Meanwhile, we still keep the option to bypass the block trigger
so users can broadcast immediately by setting `Immediate` to true.
@yyforyongyu
sweep: remove redundant error from Broadcast
9684d20
@yyforyongyu
sweep: add method handleBumpEventError and fix markInputFailed
6b32e96 
Previously in `markInputFailed`, we'd remove all inputs under the same
group via `removeExclusiveGroup`. This is wrong as when the current
sweep fails for this input, it shouldn't affect other inputs.
@yyforyongyu
sweep: add method isMature on SweeperInput
2d8ad67 
Also updated `handlePendingSweepsReq` to skip immature inputs so the
returned results are the same as those in pre-0.18.0.
@yyforyongyu
sweep: make sure defaultDeadline is derived from the mature height
e658f8e
@yyforyongyu
sweep: remove redundant loopvar assign
7dc8072
@yyforyongyu
sweep: break initialBroadcast into two steps
b1ff2fa 
With the combination of the following commit we can have a more granular
control over the bump result when handling it in the sweeper.
@yyforyongyu
sweep: make sure nil tx is handled
616399e 
After previous commit, it should be clear that the tx may be failed to
created in a `TxFailed` event. We now make sure to catch it to avoid
panic.
@yyforyongyu
chainio: introduce chainio to handle block synchronization
2bbaf6a 
This commit inits the package `chainio` and defines the interface
`Blockbeat` and `Consumer`. The `Consumer` must be implemented by other
subsystems if it requires block epoch subscription.
@yyforyongyu
chainio: implement Blockbeat
dfb5827 
In this commit, a minimal implementation of `Blockbeat` is added to
synchronize block heights, which will be used in `ChainArb`, `Sweeper`,
and `TxPublisher` so blocks are processed sequentially among them.
@yyforyongyu
chainio: add helper methods to dispatch beats
b072822 
This commit adds two methods to handle dispatching beats. These are
exported methods so other systems can send beats to their managed
subinstances.
@yyforyongyu
chainio: add BlockbeatDispatcher to dispatch blockbeats
a398cf2 
This commit adds a blockbeat dispatcher which handles sending new blocks
to all subscribed consumers.
@yyforyongyu
chainio: add partial implementation of Consumer interface
8caef2d
@yyforyongyu
multi: implement Consumer on subsystems
35dddf2 
This commit implements `Consumer` on `TxPublisher`, `UtxoSweeper`,
`ChainArbitrator` and `ChannelArbitrator`.
@yyforyongyu
sweep: remove block subscription in UtxoSweeper and TxPublisher
cf381e6 
This commit removes the independent block subscriptions in `UtxoSweeper`
and `TxPublisher`. These subsystems now listen to the `BlockbeatChan`
for new blocks.
@yyforyongyu
sweep: remove redundant notifications during shutdown
9d4a157 
This commit removes the hack introduced in lightningnetwork#4851. Previously we had this
issue because the chain notifier was stopped before the sweeper, which
was changed a while back and we now always stop the chain notifier last.
In addition, since we no longer subscribe to the block epoch chan
directly, this issue can no longer happen.
@yyforyongyu
contractcourt: remove waitForHeight in resolvers
6dd3f65 
The sweeper can handle the waiting so there's no need to wait for blocks
inside the resolvers. By offering the inputs prior to their mature
heights also guarantees the inputs with the same deadline are
aggregated.
@yyforyongyu
contractcourt: remove block subscription in chain arbitrator
bed0577 
This commit removes the block subscriptions used in `ChainArbitrator`
and replaced them with the blockbeat managed by `BlockbeatDispatcher`.
@yyforyongyu
contractcourt: remove block subscription in channel arbitrator
f092ec0 
This commit removes the block subscriptions used in `ChannelArbitrator`,
replaced them with the blockbeat managed by `BlockbeatDispatcher`.
@yyforyongyu
contractcourt: remove the immediate param used in Resolve
4ebef3f 
This `immediate` flag was added as a hack so during a restart, the
pending resolvers would offer the inputs to the sweeper and ask it to
sweep them immediately. This is no longer need due to `blockbeat`, as
now during restart, a block is always sent to all subsystems via the
flow `ChainArb` -> `ChannelArb` -> resolvers -> sweeper. Thus, when
there are pending inputs offered, they will be processed by the sweeper
immediately.
@yyforyongyu
contractcourt: start channel arbitrator with blockbeat
631e3a8 
To avoid calling GetBestBlock again.
@yyforyongyu
multi: start consumers with a starting blockbeat
9960746 
This is needed so the consumers have an initial state about the current
block.
@yyforyongyu
lnd: add new method startLowLevelServices
1f91b2b 
In this commit we start to break up the starting process into smaller
pieces, which is needed in the following commit to initialize blockbeat
consumers.
@yyforyongyu
lnd: start blockbeatDispatcher and register consumers
c01085a
@yyforyongyu
contractcourt: fix linter funlen
58a45cd 
Refactor the `Start` method to fix the linter error:
```
contractcourt/chain_arbitrator.go:568: Function 'Start' is too long (242 > 200) (funlen)
```
@yyforyongyu
multi: improve loggings
122f0db
@yyforyongyu
chainio: use errgroup to limit num of goroutines
1e17c5c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yyforyongyu @coveralls @ziggie1984 @guggero @ProofOfKeags