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

Merge new sweeper branch to master #8667

Merged
merged 121 commits into from
Apr 22, 2024
Merged

Merge new sweeper branch to master #8667

merged 121 commits into from
Apr 22, 2024

Conversation

yyforyongyu
Copy link
Member

@yyforyongyu yyforyongyu commented Apr 19, 2024

Merge the sweeper saga to master.

Fixes #8522

Fixes #8433

Fixes #4215

Fixes #6562

Fixes #6433

Fixes #8042

Fixes #7026 (via addition of budget config)

Fixes #7390

Fixes #7554

Fixes #8370

To properly reflect what the method really does. We also changes the
method signature so only a hash is used.
Which will be used to make the sweeper RBF-aware.
This commit modifies the sweeper store to save a `TxRecord` in db
instead of an empty byte slice. This record will later be used to bring
RBF-awareness to our sweeper.
Thus we can use shorter method signatures. In doing so we also remove an
old TODO in one use case of `CreateSweepTx`.
This commit refactors the sweeper so the method `feeRateForPreference`
is now moved to `FeePreference`, which makes our following refactor
easier to handle.
This commit moves `DetermineFeePerKw` into the `Estimate` method on
`FeePreference`. A few callsites previously calling `DetermineFeePerKw`
without the max fee rate is now also temporarily fixed by forcing them
to use `Estimate` with the default sweeper max fee rate.
Results from running,
```
gofmt -d -w -r 'FeePreference -> FeeEstimateInfo' .
```
This commit adds a new interface `FeePreference` which makes it easier
to write unit tests and allows more customized implementation in
following commits.
This commit refactors the grouping logic into a new interface
`UtxoAggregator`, which makes it easier to write tests and opens
possibility for future customized clustering strategies.

The old clustering logic is kept as and moved into `SimpleAggregator`.
This commit starts tracking the state of a pending input so it's easier
to control the sweeping flow and provide RBF awareness in the future.
This commit changes how a new input sweep request is handled - now we
will query the mempool and see if it's already been spent. If so, we'll
update its state as we may need to RBF this input.
This commit uniforms and put the deletion of pending inputs in a single
point.
This commit attaches RBFInfo to an input before it's been published or
it's already been published.
This commit removes the logic where we remove an input when it's been
published more than 10 times. This is needed as in our future fee
bumper, we might start with a low fee and rebroadcast the same input for
hundred of blocks.
…ublish`

Now that the refactor is done, we start patching unit tests for these
two methods. Minor changes are also made based on the feedback from the
tests.
So we can use the methods implemented on the chain RPC client.
sweeper

This commit implements a new method, `LookupInputMempoolSpend` to do
lookups in the mempool. This method is useful in the case when we only
want to know whether an input is already been spent in the mempool by
the time we call.
Thus this method `decideStateAndRBFInfo` won't touch the state changes
of a given input.
This commit adds a new check for neutrino backend - when the inputs in
the sweeping tx are spent by a third party, we will send back a
`TxFailed` event to free the rest of the inputs for re-grouping.
This commit makes sure the time-sensitive outputs are swept immediately
during startup.
This commit adds a test case to check the no deadline sweeping behavior.
The sweeper is updated to make sure it can handle the case for neutrino
backend.
This commit changes how we transform from a deadline option to a
concrete deadline value - previously this is done when we decide to
cluster inputs, and we now move it to a step earlier - once an input is
received via `SweeperInput`, we will immediately transform its optional
deadline into a real value. For inputs that come with a deadline option,
since the Some will be used, it makes no difference. For inputs with
None as their deadlines, we need this change to make sure the default
deadlines are assigned accurately.
Copy link
Contributor

coderabbitai bot commented Apr 19, 2024

Important

Auto Review Skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments
guggero
🥇
20
▀▀▀
2d 1h 59m
21
Roasbeef
🥈
9
7d 9h 40m
40
▀▀
ellemouton
🥉
9
14h 40m
13
bhandras
6
13h 56m
1
ProofOfKeags
4
7d 8h 54m
43
▀▀▀
ziggie1984
4
12d 23h 32m
36
▀▀
Crypt-iQ
2
7d 23h 33m
4
morehouse
2
12d 21h 54m
4
yyforyongyu
1
5h 15m
0
ffranr
1
3d 16h 49m
5
dstadulis
1
14d 35m
▀▀
1
bitromortac
1
12d 16h 15m
3
saubyk
1
7d 11h 5m
0
Chinwendu20
1
23h 27m
1
mohamedawnallah
1
1d 4h 44m
0

@yyforyongyu yyforyongyu force-pushed the elle-new-sweeper branch 3 times, most recently from 72c3e06 to f5decce Compare April 19, 2024 19:06
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🌊

@Roasbeef Roasbeef mentioned this pull request Apr 22, 2024
4 tasks
@Roasbeef Roasbeef merged commit 7af1957 into master Apr 22, 2024
24 of 27 checks passed
@yyforyongyu yyforyongyu deleted the elle-new-sweeper branch April 22, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment