-
Notifications
You must be signed in to change notification settings - Fork 123
sweepbatcher: threshold for immediate batch publishing #1055
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -200,6 +200,11 @@ type batchConfig struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // chainParams are the chain parameters of the chain that is used by | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // batches. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chainParams *chaincfg.Params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // immediatePublishThreshold is the cumulative value threshold above | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // which a batch should be published immediately without waiting for | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the initial delay grouping period. If 0, the feature is disabled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| immediatePublishThreshold btcutil.Amount | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // rbfCache stores data related to our last fee bump. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -807,6 +812,24 @@ func (b *batch) sweepExists(outpoint wire.OutPoint) bool { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ok | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // totalValue returns the sum of all sweep values in the batch. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a note that the method must run in the batch's event loop, so avoid races. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (b *batch) totalValue() btcutil.Amount { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var total btcutil.Amount | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, sweep := range b.sweeps { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total += sweep.value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return total | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+816
to
+822
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have variable |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // exceedsThreshold checks if the batch value exceeds the immediate publish | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // threshold. Returns false if the threshold is not configured (0). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose to add the amount as an argument or to add a note that the method must be called from the batch's event loop. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (b *batch) exceedsThreshold() bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if b.cfg.immediatePublishThreshold == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return b.totalValue() >= b.cfg.immediatePublishThreshold | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+826
to
+831
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is called to check if the threshold is exceeded, and then To optimize this, consider refactoring this function to return the total value as well, so it can be reused. This change will require updating the call site in
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Wait waits for the batch to gracefully stop. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (b *batch) Wait() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| b.Infof("Stopping") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -920,6 +943,25 @@ func (b *batch) Run(ctx context.Context) error { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skipBefore = &delayStop | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skipBeforeUpdated = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if batch exceeds threshold and should publish | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // immediately. Only override skipBefore if threshold | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // feature is enabled and exceeded. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if b.exceedsThreshold() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| b.Infof("Batch value (%v) exceeds threshold "+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "(%v), publishing immediately", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| b.totalValue(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| b.cfg.immediatePublishThreshold) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Set skipBefore to now to bypass initial | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // delay. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| now := clock.Now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skipBefore = &now | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Trigger immediate publish by setting timer to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // fire after batchPublishDelay. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timerChan = clock.TickAfter(b.cfg.batchPublishDelay) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+956
to
+963
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplification idea. I propose to move the whole branch above, under I propose also to add an empty line before |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+950
to
+964
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following my suggestion to refactor
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create new timer only if the value of skipBefore was updated. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we can implement this inside
initialDelayProvider. It already has totalSweptAmount argument, so it can have the threshold logic.