-
Notifications
You must be signed in to change notification settings - Fork 2.3k
sweep: account for aux extra budget when filtering inputs #10897
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 |
|---|---|---|
|
|
@@ -232,12 +232,47 @@ func (b *BudgetAggregator) filterInputs(inputs InputsMap) InputsMap { | |
| // https://github.com/lightning/bolts/blob/master/03-transactions.md#appendix-a-expected-weights | ||
| wu := lntypes.VByte(input.InputSize).ToWU() + witnessSize | ||
|
|
||
| // If an aux sweeper is set, it may contribute an extra budget | ||
| // to any input set this input becomes part of. The input's own | ||
| // budget may be tiny (e.g. for custom channel outputs whose | ||
| // value is mostly carried off-chain), so without accounting | ||
| // for the extra budget here we'd filter such inputs out | ||
| // permanently, even though their input set could comfortably | ||
| // pay its fees. | ||
| // | ||
| // We query per-input rather than once per round: the call | ||
| // answers "what does this input contribute to the extra | ||
| // budget of any set it joins", and is consistent with the | ||
| // once-per-set call at set construction time so long as the | ||
| // aux contribution is additive across inputs (which it is for | ||
| // the resolution-blob accounting used in practice). On a | ||
| // lookup error we fall back to zero extra budget rather than | ||
| // dropping the input, so a transient aux failure doesn't | ||
| // recreate the silently-stranded mode this guard is meant to | ||
| // avoid. | ||
| extraBudget, err := fn.MapOptionZ( | ||
| b.auxSweeper, | ||
| func(aux AuxSweeper) fn.Result[btcutil.Amount] { | ||
|
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. 🟡 Minor Still present: |
||
| return aux.ExtraBudgetForInputs( | ||
|
jtobin marked this conversation as resolved.
|
||
| []input.Input{pi.Input}, | ||
| ) | ||
|
jtobin marked this conversation as resolved.
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. 🟠 Major The new comment documents the intended semantics — |
||
| }, | ||
| ).Unpack() | ||
| if err != nil { | ||
| log.Errorf("Unable to fetch extra budget for "+ | ||
| "input=%v, falling back to own budget: %v", | ||
| op, err) | ||
|
|
||
| extraBudget = 0 | ||
| } | ||
| budget := pi.params.Budget + extraBudget | ||
|
jtobin marked this conversation as resolved.
|
||
|
|
||
| // Skip inputs that has too little budget. | ||
|
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. 🟡 Minor
|
||
| minFee := minFeeRate.FeeForWeight(wu) | ||
| if pi.params.Budget < minFee { | ||
| if budget < minFee { | ||
| log.Warnf("Skipped input=%v: has budget=%v, but the "+ | ||
| "min fee requires %v (feerate=%v), size=%v", op, | ||
| pi.params.Budget, minFee, | ||
| budget, minFee, | ||
| minFeeRate.FeePerVByte(), wu.ToVB()) | ||
|
|
||
| continue | ||
|
|
@@ -248,10 +283,10 @@ func (b *BudgetAggregator) filterInputs(inputs InputsMap) InputsMap { | |
| chainfee.SatPerKWeight(0), | ||
| ) | ||
| startingFee := startingFeeRate.FeeForWeight(wu) | ||
| if pi.params.Budget < startingFee { | ||
| if budget < startingFee { | ||
| log.Errorf("Skipped input=%v: has budget=%v, but the "+ | ||
| "starting fee requires %v (feerate=%v), "+ | ||
| "size=%v", op, pi.params.Budget, startingFee, | ||
| "size=%v", op, budget, startingFee, | ||
| startingFeeRate.FeePerVByte(), wu.ToVB()) | ||
|
|
||
| continue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,108 @@ | |
| require.Contains(t, result, opHigh) | ||
| } | ||
|
|
||
| // TestBudgetAggregatorFilterInputsAuxBudget checks that the aux sweeper's | ||
| // extra budget is folded into the filter's budget check, and that an aux | ||
| // lookup failure falls back to gating on the input's own budget rather than | ||
| // silently dropping the input. | ||
| func TestBudgetAggregatorFilterInputsAuxBudget(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| const wu lntypes.WeightUnit = 100 | ||
| inpSize := lntypes.VByte(input.InputSize).ToWU() + wu | ||
|
|
||
| const minFeeRate = chainfee.SatPerKWeight(1000) | ||
| minFee := minFeeRate.FeeForWeight(inpSize) | ||
|
|
||
| // shortfall is how much the own budget falls short of minFee; the aux | ||
| // sweeper covers exactly this gap in the "rescue" cases. | ||
| const shortfall = btcutil.Amount(100) | ||
| auxErr := errors.New("aux failure") | ||
|
|
||
| testCases := []struct { | ||
| name string | ||
| ownBudget btcutil.Amount | ||
| auxResult fn.Result[btcutil.Amount] | ||
| expectKept bool | ||
| }{ | ||
| { | ||
| // The input's own budget falls short of the min fee, | ||
| // but the aux sweeper contributes enough extra budget | ||
| // to clear it. Pre-fix this input would have been | ||
| // filtered out. | ||
| name: "aux budget rescues low-own-budget input", | ||
| ownBudget: minFee - shortfall, | ||
| auxResult: fn.Ok(shortfall), | ||
| expectKept: true, | ||
| }, | ||
| { | ||
| // The aux lookup errors but the input's own budget | ||
| // already covers the min fee, so the conservative | ||
| // fallback (extraBudget=0) keeps it in. Pre-fix this | ||
| // input would have been silently dropped. | ||
| name: "aux error keeps own-budget-sufficient input", | ||
| ownBudget: minFee, | ||
| auxResult: fn.Err[btcutil.Amount](auxErr), | ||
| expectKept: true, | ||
| }, | ||
| { | ||
| // The aux lookup errors and the input cannot pay its | ||
| // own way, so it is correctly filtered. | ||
| name: "aux error drops below-min-fee input", | ||
| ownBudget: minFee - shortfall, | ||
| auxResult: fn.Err[btcutil.Amount](auxErr), | ||
| expectKept: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| estimator := &chainfee.MockEstimator{} | ||
| defer estimator.AssertExpectations(t) | ||
| estimator.On("RelayFeePerKW").Return(minFeeRate).Once() | ||
|
|
||
| wt := &input.MockWitnessType{} | ||
| defer wt.AssertExpectations(t) | ||
| wt.On("SizeUpperBound").Return(wu, true, nil).Once() | ||
|
|
||
| mockInput := &input.MockInput{} | ||
| defer mockInput.AssertExpectations(t) | ||
| op := wire.OutPoint{Hash: chainhash.Hash{1}} | ||
| mockInput.On("WitnessType").Return(wt) | ||
| mockInput.On("OutPoint").Return(op) | ||
| if tc.expectKept { | ||
| mockInput.On("RequiredTxOut").Return(nil) | ||
|
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. 🟡 Minor In |
||
| } | ||
|
|
||
| mockAux := &MockAuxSweeper{} | ||
| defer mockAux.AssertExpectations(t) | ||
| mockAux.On("ExtraBudgetForInputs").Return(tc.auxResult) | ||
|
|
||
| inputs := InputsMap{ | ||
| op: &SweeperInput{ | ||
| Input: mockInput, | ||
| params: Params{Budget: tc.ownBudget}, | ||
| }, | ||
| } | ||
|
|
||
| b := NewBudgetAggregator( | ||
| estimator, 0, | ||
| fn.Some[AuxSweeper](mockAux), | ||
| ) | ||
| result := b.filterInputs(inputs) | ||
|
|
||
| if tc.expectKept { | ||
| require.Contains(t, result, op) | ||
| } else { | ||
| require.NotContains(t, result, op) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestBudgetAggregatorSortInputs checks that inputs are sorted by based on | ||
| // their budgets and force flag. | ||
| func TestBudgetAggregatorSortInputs(t *testing.T) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.