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

btc: more sensible bond fee buffer #2382

Merged
merged 2 commits into from
Jun 16, 2023
Merged

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Jun 13, 2023

When used for computing the bond fee buffer, the "fee rate limit" makes
an extremely high fee rate buffer, which makes BTC bonds impractical.
This change:
- switches to using a dynamic fee rate with the configured fallback rate
  for both ReserveBondFunds and BondsFeeBuffer, where the former is
  used as the requirement for the frontend to proceed with registration
- uses the exported BondsFeeBuffer method when automatically creating
  the "extra" split output in certain order funding edge cases instead
  of a buffer based on the high limit.
- tweaks the bond tx input count to be slightly less conservative, going
  from 12 -> 8 possible inputs

The motivation for these changes is that using the wallet's configured
fee rate limit creates extremely high fee buffers, whereas we have
defined the "fallback" fee rate to be a reasonable fallback where a
current estimate is not available, which also happens to be applicable
to a *future* fee rate.

Further, this is a buffer and precision is not critical, but turning
users away from on-boarding is a major issue.  The right approach
is to base reserves on a padded estimate based on current rates,
not the swap fee rate limit.

Finally, given this softening of the reserves amount, no adjustment to
the enforced bond reserves in Reconfigure is done since we are now
using current rates as a hit of future rates.

Padding of this fee buffer already comes from:
- planning for a lot of inputs
- 4 parallel bond tracks
- the exported BondsFeeBuffer method using double the provided fee rate

Even if fee rates skyrocket, and the reserves become insufficient for
bond renewal, the user will still post bond if their balance allows, and
they may deposit some nominal additional amount to cover any such
deficit.

Before

image

(that's about 1500 USD, not sensible)

After

image

(that's <100 USD, or 2x the bond amount plus realistic fees)

@chappjc chappjc added this to the 0.6.2 milestone Jun 13, 2023
@chappjc chappjc changed the title btc: more sensible bond fee buffer [0.6.2] btc: more sensible bond fee buffer Jun 13, 2023
@chappjc chappjc requested a review from buck54321 June 14, 2023 19:38
}
btc.reservesMtx.Unlock()
}
btc.cfgV.Store(newCfg) // probably won't matter if restart/reinit required
Copy link
Member Author

Choose a reason for hiding this comment

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

Back to not touching the bond reserves on wallet config change.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

How about a feeSuggestion uint64 argument to BondsFeeBuffer and ReserveBondFunds so that wallets without external API enabled will still get realistic values, even if the user has misconfigured their fallback rate?

@chappjc chappjc force-pushed the btc-bond-fee-buffer branch from 0a6362e to 892117f Compare June 14, 2023 20:07
@chappjc

This comment was marked as resolved.

@chappjc

This comment was marked as resolved.

@chappjc

This comment was marked as resolved.

@chappjc chappjc changed the title [0.6.2] btc: more sensible bond fee buffer btc: more sensible bond fee buffer Jun 14, 2023
@chappjc
Copy link
Member Author

chappjc commented Jun 14, 2023

Refactored so that the BondsFeeBuffer and ReserveBondFunds methods now take a fee rate. If zero, those methods will try to get an internal rate or use the configured fallback.

@chappjc
Copy link
Member Author

chappjc commented Jun 15, 2023

Will push a refactor shortly. Sorry if there are reviews in progress.

@chappjc chappjc marked this pull request as draft June 15, 2023 23:08
@chappjc chappjc force-pushed the btc-bond-fee-buffer branch 3 times, most recently from 4116e95 to 4a7c7cf Compare June 16, 2023 00:18
@chappjc
Copy link
Member Author

chappjc commented Jun 16, 2023

OK, despite now using a live fee estimate as the basis for the bonds fee buffer, the wallet wait form should not be fickle with respect to the required amount to proceed. Also, the (*Core).PostBond method now has an optional fee buffer field in it's parameter struct so that the fee buffer that was planned for will be used when setting the reserves.

Last commit fixes an existing js bug where the wallet wait form would be skipped entirely if the wallet existed and had at least one bond amount in available balance (not the required double plus fee buffer).

@chappjc chappjc marked this pull request as ready for review June 16, 2023 00:23
@chappjc chappjc removed this from the 0.6.2 milestone Jun 16, 2023
chappjc added 2 commits June 16, 2023 13:04
When used for computing the bond fee buffer, the "fee rate limit" makes
an extremely hight fee rate buffer, which makes BTC bonds impractical.
This change:
- switches to using a dynamic fee rate with the configured fallback rate
  for both ReserveBondFunds and BondsFeeBuffer, where the former is
  used as the requirement for the frontend to proceed with registration
- uses the exported BondsFeeBuffer method when automatically creating
  the "extra" split output in certain order funding edge cases instead
  of a buffer based on the high limit.
- tweaks the bond tx input count to be slightly less conservative, going
  from 12 -> 8 possible inputs

The motivation for these changes is that using the wallet's configure
fee rate limit create extremely high fee bufferes, whereas we have
defined the "fallback" fee rate to be a reasonable fallback where a
current estimate is not available.

Further, this is a buffer and precision is not critical, but turning
users away from onboarding is a major issue.  The right approach
is to base reserves on a padded estimate based on current rates,
not the swap fee rate limit.

Finally, given this softening of the reserves amount, no adjustment to
the enforced bond reserves in Reconfigure is done since we are now
using current rates as a hit of future rates.

Padding of this fee buffer already comes from:
- planning for a lot of inputs
- 4 parallel bond tracks
- the exported BondsFeeBuffer method reporting 110% of the above

Even if fee rates skyrocket, and the reserves become insufficient for
bond renewal, the user will still post bond if their balance allows, and
they may deposit some nominal additional amount to cover any such
deficit.
@chappjc chappjc force-pushed the btc-bond-fee-buffer branch from 4a7c7cf to 1470f97 Compare June 16, 2023 18:04
@chappjc chappjc merged commit 36ba3ae into decred:master Jun 16, 2023
@chappjc chappjc deleted the btc-bond-fee-buffer branch June 16, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants