client: bond reserves #2103
client: bond reserves #2103
Conversation
client/asset/dcr/coin_selection.go
Outdated
| return nil | ||
| } | ||
| if i == len(utxos)-1 { | ||
| return utxos[i:] |
There was a problem hiding this comment.
This should never happen if as in the comment, Each utxo in the input must be smaller than amt
| @@ -3555,10 +3618,16 @@ func (dcr *ExchangeWallet) RefundBond(ctx context.Context, ver uint16, coinID, s | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| return dcr.makeBondRefundTxV0(txHash, vout, amt, script, privKey, feeRate) | |||
|
|
|||
| newReserves, unspent := dcr.bondSpent(amt) // nominal, not refundAmt | |||
There was a problem hiding this comment.
You could call bondSpent after the SendRawTransaction, instead of re-locking in the error block.
chappjc
force-pushed
the
bond-reserves
branch
from
819f51a
to
ffa9d43
Compare
|
Just a rebase, minimal review revisions |
chappjc
force-pushed
the
bond-reserves
branch
from
0e01dc9
to
dd58c2e
Compare
| @@ -916,10 +1099,16 @@ func (c *Core) PostBond(form *PostBondForm) (*PostBondResult, error) { | |||
| "Consider using UpdateBondOptions instead.", | |||
| targetTier, autoBondAsset, wallet.amtString(maxBondedAmt)) | |||
| } else if maintain { // new account with tier maintenance enabled | |||
There was a problem hiding this comment.
Unrelated to review, but I think there should be a "force" flag for someone to call this when the account already exists.
client/core/bond.go
Outdated
| if tierOffset := int64(bondedTier()) - dc.acct.tier; tierOffset > 0 { | ||
| mod += tierOffset * int64(bondAssetAmt) | ||
| } | ||
| // Wallet already has live unspent bonds (inBonds) register. |
| if tierChanged { | ||
| dc.acct.targetTier = targetTier | ||
| acct.TargetTier = targetTier | ||
| } |
There was a problem hiding this comment.
Could return early if !tierChanged && !assetChanged
There was a problem hiding this comment.
It's permitted to change max bonded amt only.
| enforcedDelta := future + int64(feeBuffer) | ||
|
|
||
| // How much of that is covered by the available balance | ||
| if respectBalance { |
There was a problem hiding this comment.
Maybe only check this if future > dcr.bondReservesNominal? If someone's decreasing the reserves and their balance isn't high enough it would mean that something went wrong somewhere else, but I don't think it should fail here.
There was a problem hiding this comment.
I think future > 0 would be the condition to indicate if reserves are increasing.
client/core/core.go
Outdated
| @@ -4803,7 +4836,7 @@ func (c *Core) MaxSell(host string, base, quote uint32) (*MaxOrderEstimate, erro | |||
| } | |||
|
|
|||
| // initializeDEXConnections connects to the DEX servers in the conns map and | |||
| // authenticates the connection. This is done on Login. | |||
| // authenticates the connection. This should be done only ONCE on Login. | |||
There was a problem hiding this comment.
This is also called in AccountImport.
There was a problem hiding this comment.
Ah right, that's an atrocity. Going to change that. I'm also going to remove connectAccount now.
There was a problem hiding this comment.
Oh heck, the dc.acct.authed() check in here makes this safe, so I'll remove the comment for now, but I figure we should factor out the loop contents for stuff like AccountImport.
client/asset/dcr/dcr.go
Outdated
| // unspent bonds, which are in bondReservesUsed. When bonds are created, | ||
| // bondReservesEnforced is decremented and bondReservesUsed are incremented; | ||
| // when bonds are refunded, the reverse. bondReservesEnforced may become | ||
| // negative: during the unbonding process, if there is insufficient balance |
There was a problem hiding this comment.
Wouldn't insufficient balance due to penalties and pre-reserving more than available just make bondReservesEnforced be greater than the balance, not become negative?
| // after excess is unbonded, offsetting the negative enforced amount. This | ||
| // is the relatively small fee buffer. | ||
| if int64(dcr.bondReservesUsed) == dcr.bondReservesNominal { | ||
| return uint64(-dcr.bondReservesEnforced) |
There was a problem hiding this comment.
Should the fee buffer be updated on reconfigure, if the feeRateLimit is changed?
| // Find a subset of the small UTXO set with smallest combined amount. | ||
| var set []*compositeUTXO | ||
| if sumUTXOs(small) >= amt { | ||
| set = subsetLargeBias(amt, small) |
There was a problem hiding this comment.
Did you consider a dynamic programming technique to find the optimal solution?
There was a problem hiding this comment.
Was on my todo list. 😊 Just threw together a few heuristics, but you have a point that it can be solved optimally. Seems akin to the fractional knapsack problem (reversed). Will give it a shot.
OK, will have to go back to test fresh accounts. I did some revisions to the system that I validated with existing accounts, but must have regressed with account creation. |
Oopsie fixed in 638214d |
chappjc
force-pushed
the
bond-reserves
branch
from
e1fbcba
to
d3e362e
Compare
client/asset/dcr/dcr.go
Outdated
| // Try again with some utxos taken out of the mix for keep. Select these | ||
| // with the objective of being as close to the amount as possible, | ||
| // unlike tryFund that minimizes the number of UTXOs chosen. | ||
| kept := leastOverFund(keep, utxos) | ||
| utxos = utxoSetDiff(utxos, kept) | ||
| sum, _, sz, coins, spents, redeemScripts, err = tryFund(utxos, enough) |
There was a problem hiding this comment.
It's not clear to me exactly how this might work. avail-sum+extra will be all that remains after the tx is funded, including the change transaction. Is the hope that if we reserve the right utxos bond reserves, tryFund will somehow find a more favorable set of utxos?
There was a problem hiding this comment.
If we get here, it is because the enough func is very likely not granting any spendable change i.e. no split tx. Note the if extra != 0 { check above. The logic in this commit is to try again but fist pick out utxos in the amount of at least keep before even calling tryFund -- that guarantees keep will be met but it can then fail if the pruned utxo set is insufficient for the order's enough func.
However (and I apologize for the moving target), after we chatted on matrix this morning, I switched the order of these attempts so that it first sets aside utxos (using just the pruned set with tryFund), and if that fails to leave enough funding, it will then try with the full set and check if extra allowed it to satisfy keep: 9f6ad97
| // if sum >= amt { return utxos } // would have been i>=0 after break above | ||
| return nil | ||
| } | ||
| // if i == len(utxos)-1 { return utxos[i:] } // shouldn't happen if each in set are < amt |
There was a problem hiding this comment.
Hmm. if i == len(utxos)-1 you wouldn't return the last one though.
There was a problem hiding this comment.
It's commented out here of course, but I'm not sure why I had written this as utxos[i:], because that's the same as utxos[i] when i == len(utxos)-1 (the last one). As @martonp pointed out, it's not going to happen that just the one UTXO is sufficient because: Each utxo in the input must be smaller than amt - use via leastOverFund only! so this is commented. Might as well remove.
| // large enough to fully fund the requested amount, if it exists. If the smaller | ||
| // set is insufficient, the single largest UTXO is returned. If instead the set |
There was a problem hiding this comment.
Does "smaller set" mean the set of lower valued utxos?
client/asset/dcr/dcr.go
Outdated
| } | ||
|
|
||
| func bondsFeeBuffer(highFeeRate uint64) uint64 { | ||
| const inputCount uint64 = 20 // plan for lots of inputs |
There was a problem hiding this comment.
20 seems like a lot. Won't matter much for low-fee assets, but might be prohibitive for btc and eth at times.
There was a problem hiding this comment.
Probably true in most cases, esp. with the high fee rate and the "tracks" multiplier below, but this helper is in client/asset/dcr so other assets would do differently. I expect eth would be quite a bit simpler in this regard (no UTXO business to worry about), although this could be a lot of value for btc.
client/asset/dcr/dcr.go
Outdated
|
|
||
| func bondsFeeBuffer(highFeeRate uint64) uint64 { | ||
| const inputCount uint64 = 20 // plan for lots of inputs | ||
| highBondFee := dexdcr.MsgTxOverhead + dexdcr.P2PKHOutputSize*2 + inputCount*dexdcr.P2PKHInputSize |
There was a problem hiding this comment.
Output size could be more accurate. 1 p2sh, 1 OP_RETURN + push data length, and 1 p2pkh
const BondPushDataSize = 1 + 2 + 32 + 4 + 20
highBondFee := dexdcr.MsgTxOverhead + dexdcr.P2SHOutputSize + BondPushDataSize + dexdcr.P2PKHOutputSize + inputCount*dexdcr.P2PKHInputSize
client/asset/dcr/dcr.go
Outdated
|
|
||
| func bondsFeeBuffer(highFeeRate uint64) uint64 { | ||
| const inputCount uint64 = 20 // plan for lots of inputs | ||
| highBondFee := dexdcr.MsgTxOverhead + dexdcr.P2PKHOutputSize*2 + inputCount*dexdcr.P2PKHInputSize |
There was a problem hiding this comment.
Variable name could indicate size.
| // NOTE: Split tx is an order-time option. The max order is generally | ||
| // attainable when split is used, regardless of whether they choose it on | ||
| // the order form. Allow the split for max order purposes. | ||
| trySplitTx := true |
There was a problem hiding this comment.
The max order is generally attainable when split is used, regardless of whether they choose it on the order form.
Really? I don't see it.
There was a problem hiding this comment.
Yeah, it's more nuanced than I let on, but this is referring to when there are reserves to respect, because the split maximizes the usable balance by allowing the split tx change to offset reserves.
However, the split actually eats a bit extra in fees because of the split tx baggage, but because dcr.estimateSwap falls back to skipping the split if needed, we can be sure that maxOrder calling estimateSwap with trySplitTx=true will give the highest estimate one way or another. The comments in the revised tests in 57db9ad demonstrate this I think.
client/asset/dcr/dcr.go
Outdated
| estHighFunds := calc.RequiredOrderFundsAlt(val, uint64(inputsSize), lots, | ||
| dexdcr.InitTxSizeBase, dexdcr.InitTxSize, bumpedNetRate) | ||
| estHighFees := estHighFunds - val | ||
| digestInputs := func() (reqFunds, maxFees, estHighFees, estLowFees uint64) { |
There was a problem hiding this comment.
Making inputsSize an argument might be more descriptive.
| } | ||
| // Always offer the split option, even for non-standing orders since | ||
| // immediately spendable change many be desirable regardless. | ||
| opts := []*asset.OrderOption{dcr.splitOption(req, utxos, bump)} |
There was a problem hiding this comment.
Ah. Nice. I had commented on this on a different commit but hadn't submitted yet.
It looks like we're still disabling the split in FundOrder for Immediate orders though.
| // loss to fees in a split. This trivial amount is of no concern because | ||
| // the reserves should be buffered for amounts much larger than the fees | ||
| // on a single transaction. |
There was a problem hiding this comment.
I think you're probably right that the fee buffer will cover it, but I don't see any harm in adding a dexdcr.P2PKHOutputSize to the splitTxBaggage above if extraOutput > 0.
chappjc
force-pushed
the
bond-reserves
branch
2 times, most recently
from
fd53f71
to
9a16a76
Compare
client/asset/dcr/dcr.go
Outdated
| // balance. Amounts may be reserved beyond the available balance, but only the | ||
| // amount that is offset by the available balance is reflected in the locked | ||
| // balance category. Like funds locked in swap contracts, the caller must | ||
| // supplement balance reporting with know bond amounts. However, via |
| // the reserves are decremented to zero (by the amount that they were | ||
| // incremented), all enforcement including any fee buffering is disabled. | ||
| ReserveBondFunds(future int64, respectBalance bool) bool | ||
|
|
There was a problem hiding this comment.
One thing I think is going to be needed is a way to check that reserves are sufficient without ordering or sending. We need to be able to provide warnings to users when there's insufficient funds, which could be caused by e.g. user doing dumb stuff with an RPC wallet, or a fee rate spike.
There was a problem hiding this comment.
Good point, this also happens if you sweep the max out of your wallet (leaving 0 "available" and just the reserves in "locked"), then on the next bond tx, it starts eating into the desired fee buffer. Of course, that's the fee buffer doing it's job and you can go on for quite a long time before it is unable to post/renew, but you can observe it running below target reserves.
e.g.
[WRN] CORE[dcr]: Available balance is below configured reserves: 0.013529 < 0.013628
and
[WRN] CORE[dcr]: Available balance is below configured reserves: 10.013479 < 10.013628
But like you said the user can meddle with their wallet to really eat into the reserves way past the fee buffer.
I think we can add a method to report reserves health.
There was a problem hiding this comment.
We have that unused asset.Balance.Other field.
I expect frontend can work with the following or similar?
@@ -1061,7 +1062,9 @@ func (dcr *ExchangeWallet) Balance() (*asset.Balance, error) {
if reserves > bal.Available { // unmixed (immature) probably needs to trickle in
dcr.log.Warnf("Available balance is below configured reserves: %f < %f",
toDCR(bal.Available), toDCR(reserves))
+ bal.Other["Reserves Deficit"] = reserves - bal.Available
reserves = bal.Available
}
+ bal.Other["Reserves"] = reserves
bal.Available -= reserves
bal.Locked += reservesOr keys can be prefixed like bal.Other["warning:Reserves Deficit"] = reserves - bal.Available so it can be shown in red or something?
There was a problem hiding this comment.
I this could work out
I'd update that with Bonds (locked) too for completeness
There was a problem hiding this comment.
I kinda like this, in the last commit
later, while not swapping and with bond overlap:
There was a problem hiding this comment.
Maybe Bonded and Bond Reserves. Someone might not know what the reserves are for.
| const parallelTracks uint64 = 4 | ||
| return parallelTracks * largeBondTxSize * highFeeRate |
There was a problem hiding this comment.
I guess there'd be no way to "merge" parallel tracks without going up or down in tier. That's unfortunate.
There was a problem hiding this comment.
I need to really research this, but I observed them naturally merging sometimes. Perhaps only likely with the high overlap ratio in simnet and testnet though.
client/asset/dcr/dcr.go
Outdated
| // bondReservesEnforced is used to reserve unspent amounts for upcoming bond | ||
| // transactions, and does not include amounts that are currently locked in | ||
| // unspent bonds, which are in bondReservesUsed. When bonds are created, | ||
| // bondReservesEnforced is decremented and bondReservesUsed are incremented; | ||
| // when bonds are refunded, the reverse. bondReservesEnforced may become | ||
| // negative during the unbonding process. | ||
| bondReservesEnforced int64 // set by ReserveBondFunds, modified by bondSpent and bondLocked |
There was a problem hiding this comment.
I'd mention that bondReservesEnforced includes projected fees.
client/core/bond.go
Outdated
| continue | ||
| } | ||
| refundCoinStr, _ = asset.DecodeCoinID(bond.AssetID, refundCoinID) | ||
| } else { // normal pat |
| } | ||
| c.log.Infof("Updating bond reserves by %s (automatic tier change adjustments)", | ||
| wallet.amtStringSigned(reserveDelta)) | ||
| wallet.ReserveBondFunds(reserveDelta, false) |
There was a problem hiding this comment.
Is it possible for rotateBonds to run concurrently?
There was a problem hiding this comment.
Should only be run from the loop in watchBonds (sequentially). Will document that it's not to be run willy nilly.
Much appreciated. It is a lot of work to digest. |
chappjc
force-pushed
the
bond-reserves
branch
2 times, most recently
from
26b5999
to
0a65957
Compare
|
Rebased and squashed away fixup commits, not retested yet, but there might be breakage with view-only merged. Compared with original PR, there are three additional commits with qualitative changes beyond fixups to the other commits. Minimal testing of the forced split with the extra output. |
client/asset/dcr/dcr.go
Outdated
| // First take some UTXOs out of the mix for any keep amount. Select these | ||
| // with the objective of being as close to the amount as possible, unlike | ||
| // tryFund that minimizes the number of UTXOs chosen. By doing this first, | ||
| // we may be making the order spend a larger number of UTXOs, but we | ||
| // mitigate subsequent order funding failure due to reserves because we know | ||
| // this order will leave behind sufficient UTXOs without relying on change. | ||
| if keep > 0 { | ||
| kept := leastOverFund(keep, utxos) | ||
| utxos = utxoSetDiff(utxos, kept) | ||
| sum, _, sz, coins, spents, redeemScripts, err = tryFund(utxos, enough) | ||
| if err != nil { // no joy with the reduced set | ||
| dcr.log.Debugf("Setting aside %v DCR in %d UTXOs to respect the %v DCR reserved amount", | ||
| toDCR(sumUTXOs(kept)), len(kept), toDCR(keep)) | ||
| utxosPruned := utxoSetDiff(utxos, kept) | ||
| sum, _, sz, coins, spents, redeemScripts, err = tryFund(utxosPruned, enough) | ||
| if err != nil { // try with the full set | ||
| dcr.log.Debug("Unable to fund order with UTXOs set aside (%v), trying again with full UTXO set.", err) | ||
| } // else spents is populated | ||
| } | ||
| if len(spents) == 0 { // either keep is zero or it failed with utxosPruned | ||
| // Without utxos set aside for keep, we have to consider any spendable | ||
| // change (extra) that the enough func grants us. | ||
| var extra uint64 | ||
| sum, extra, sz, coins, spents, redeemScripts, err = tryFund(utxos, enough) |
There was a problem hiding this comment.
This commit also reverses the order of funding attempts vs the initial commit that updating fund/tryFund. It tries straight away to set aside UTXOs for the reserves, and falls back to the standard funding approach, hoping that the enough func grants usable change in the extra output.
chappjc
force-pushed
the
bond-reserves
branch
from
0a65957
to
5cdc07e
Compare
|
Tested forced split with extra output. Worked as intended. |
chappjc
force-pushed
the
bond-reserves
branch
from
5cdc07e
to
7e67638
Compare
|
Just a rebase. Nothing else revised. |
chappjc
force-pushed
the
bond-reserves
branch
from
7e67638
to
9b3d7be
Compare
| useSplit = true | ||
| coins, redeemScripts, sum, inputsSize, err = dcr.fund(reserves, | ||
| orderEnough(ord.Value, ord.MaxSwapCount, bumpedMaxRate, true)) | ||
| // And make an extra output for the reserves amount plus additional |
There was a problem hiding this comment.
You could do reserves + (bumpedMaxRate * dexdcr.P2PKHInputSize) as the keep argument to fund so it will fail here.
There was a problem hiding this comment.
Suggestion applied and squashed in.
| useSplit := cfg.useSplitTx | ||
| if customCfg.Split != nil { | ||
| useSplit = *customCfg.Split | ||
| } | ||
|
|
||
| // Send a split, if preferred. | ||
| if useSplit && !ord.Immediate { | ||
| changeForReserves := useSplit && cfg.unmixedAccount == "" |
There was a problem hiding this comment.
Why only if cfg.unmixedAccount == "" ? The splitting outputs go into the tradingAccount.
There was a problem hiding this comment.
Just the non-change outputs. The change output goes into the unmixed account, according to signTxAndAddChange+depositAccount.
Mixing epochs are unfortunately kinda long so I don't think we can consider such change as available
There was a problem hiding this comment.
I see now.. but why does that change need to go into the unmixed account when we are doing a split? Couldn't it go right back into the trading account?
There was a problem hiding this comment.
It's debatable, but all the input have been linked with each other. They're also now linked with a dex order.
There was a problem hiding this comment.
Wouldn't the second output created for the reserves have the same issue then?
There was a problem hiding this comment.
Yeah, and same as when an order that's funded by a split output is cancelled with no fill, that output stays in trading account.
But if we send nothing into unmixed, a wallet will quickly end up with nothing in the mixed account, I think. Meaning the entire previously mixed set gets linked. It would be down to deposits, redeems, and change on matched orders. And the last category would not exist with splits in use.
chappjc
force-pushed
the
bond-reserves
branch
from
548f587
to
a7016f3
Compare
|
Ugh, the browser goes back to /register after restarting even if you've been bonded. Something with view-only logic. |
|
Oh nevermind I just deleted my dexc.db while it was running! bad chappjc |
|
I'm done with reviews on this, I think. Only thing I have to check is what Wisdom discovered with legacy accounts, which is by no means a showstopper, but should be fixed:
|
chappjc
force-pushed
the
bond-reserves
branch
from
dac923e
to
479667e
Compare
Seems the missing condition was this 479667e Will test it out though. |
chappjc
force-pushed
the
bond-reserves
branch
from
479667e
to
91932b1
Compare
client/asset/dcr/dcr.go
Outdated
| // How much of that is covered by the available balance, when increasing | ||
| // reserves via |
There was a problem hiding this comment.
Is this a complete thought?
client/asset/dcr/dcr.go
Outdated
| // else we got lucky with the legacy funding approach and there was | ||
| // either available unspent or the enough func granted spendable change. | ||
| if keep > 0 && extra > 0 { | ||
| dcr.log.Debugf("Funding succeeded with %s DCR in spendable change.", toDCR(extra)) |
There was a problem hiding this comment.
prints 2023-02-22 09:40:16.084 [DBG] CORE[dcr]: Funding succeeded with %!s(float64=42.00919958) DCR in spendable change.
client/asset/dcr/dcr.go
Outdated
|
|
||
| // Calculate the extra fees associated with the additional inputs, outputs, | ||
| // and transaction overhead, and compare to the excess that would be locked. | ||
| baggageFees := bumpedMaxRate * splitTxBaggage | ||
| if extraOutput > 0 { | ||
| baggageFees += dexdcr.P2PKHOutputSize |
There was a problem hiding this comment.
dexdcr.P2PKHOutputSize * bumpedMaxRate?
| func (dcr *ExchangeWallet) sendCoins(coins asset.Coins, addr, addr2 stdaddr.Address, val, val2, feeRate uint64, | ||
| subtract bool) (*wire.MsgTx, uint64, error) { |
There was a problem hiding this comment.
Can you add to the comment the purpose of the second addr and val?
client/core/bond.go
Outdated
| if bond.KeyIndex == math.MaxUint32 { // invalid/unknown key index fallback (v0 db.Bond, which was never released), also will skirt reserves :/ | ||
| refundCoinID, err := wallet.SendTransaction(bond.RefundTx) | ||
| if err != nil { | ||
| c.log.Errorf("Failed to broadcast bond refund txn %v: %v", bond.RefundTx, err) |
client/core/wallet.go
Outdated
| } | ||
|
|
||
| // RegisterUnspent informs the wallet of existing bond amounts that will be | ||
| // refunded with RefundBond. If reserves are subsequently been enable with |
chappjc
force-pushed
the
bond-reserves
branch
2 times, most recently
from
4c723b4
to
9c3ff6f
Compare
| @@ -138,7 +138,7 @@ var ( | |||
| "mining fees are paid. Used only for standing-type orders, e.g. " + | |||
| "limit orders without immediate time-in-force.", | |||
| IsBoolean: true, | |||
| DefaultValue: false, | |||
| DefaultValue: true, // cheap fees, helpful for bond reserves, and adjustable at order-time | |||
There was a problem hiding this comment.
Option description needs to be updated to not say it is only for standing-type orders.
chappjc
force-pushed
the
bond-reserves
branch
from
9c3ff6f
to
c23ad60
Compare
The asset.Bonder.RefundBond method now broadcasts, as it was supposed to from the beginning. Only MakeBondTx required broadcasting in the consumer.
This adds the client.WalletBalance.BondLocked field, which parallels the ContractLocked field. These are used for core to supplement the wallet's reported balance with values that the wallet does not include in the balance itself because they are not spendable by the wallet alone (core authors the spending tx). This also updates the web UI's TypeScript to include bondLocked when appropriate. This also fixes a bug where contractLocked was not included on the wallets page's "Locked" balance category. This also adds to DCR's implementation fields for reserves tracking. The values are included in the reported asset.Balance. Subsequent commits will add the ability modify the reserves, and for funding methods to respect the reserves.
This updates the DCR wallet's transaction funding method and helpers to respect the reserves added in the previous commit. To do so without creating sizing transactions to actively isolate the reserved value, we add new coin selection functions that we use exclude UTXOs from transaction funding. The objective of these coin selection functions is different from order funding coin selection. The main helper, leastOverFund, attempts to pick a subset of the provided UTXOs to reach the required amount with the objective of minimizing the total amount of the selected UTXOs. This is different from the objective used when funding orders, which is to minimize the number of UTXOs (to minimize fees). NOTE: The provided UTXO set MUST be sorted in ascending order (smallest first, largest last)! leastOverFund begins by partitioning the UTXO slice before the smallest single UTXO that is large enough to fully fund the requested amount, if it exists. If the smaller set is insufficient, the single largest UTXO is returned. If instead the set of smaller UTXOs has enough total value, it will search for a subset that reaches the amount with least over-funding (see subsetSmallBias and subsetLargeBias). If that subset has less combined value than the single sufficiently-large UTXO (if it exists), the subset will be returned, otherwise the single UTXO will be returned. We also modify the "enough" functions and their constructors with excess (change) reporting in a new return from the enough function. The constructor also accepts a flag indicating if this returned value should be set to zero. The tryFund helper now returns the final excess value when it completes UTXO selection. This is used in the (*ExchangeWallet).fund method, and in other callers, to discern if there would be sufficient remaining in the wallet (including this change) to satisfy a reserved amount. This amount is an input to fund called "keep". If change should not be considered "kept" (e.g. no preceding split txn, or mixing sends change to umixed account where it is unusable for reserves), caller should return 0 extra from the enough func (via the enough func's constructor flag).
This updates DCR's backend to begin modifying the reserves fields as bonds are created (locked UTXOs) and spend (unlock UTXOs into balance). Subsequent commits add the new exported wallet interface methods for a consumer (core) to prepare and adjust the reserves for the client's needs. This also modifies the wallet's MakeBondTx method signature with an additional return, an "abandon" function" to be used if the consumer decides not to broadcast the generated bond transaction. This function both unlocks the bond's funding utxos, and reverts the reserves updates that were made when the transaction was created.
This adds the new Bonder methods, RegisterUnspent and ReserveBondFunds: RegisterUnspent informs the wallet of a certain amount already locked in unspent bonds that will eventually be refunded with RefundBond. This should be used prior to ReserveBondFunds. This alone does not enable reserves enforcement, and it should be called on bring-up when existing bonds that may refund to this wallet are known. Once ReserveBondFunds is called, even with 0 for future bonds, these live bond amounts will become enforced reserves when they are refunded via RefundBond. ReserveBondFunds (un)reserves funds for creation of future bonds. MakeBondTx will create transactions that decrement these reserves, while RefundBond will replenish the reserves. If the wallet's available balance should be respected when adding reserves, the boolean argument may be set to indicate this, in which case the return value indicates if it was able to reserve the funds. In this manner, funds may be pre-reserved so that when the wallet receives funds (from either external deposits or refunding of live bonds), they will go directly into locked balance. When the reserves are decremented to zero (by the amount that they were incremented), all enforcement including any fee buffering is disabled. Previously registered unspent/active bonds are not forgotten however; they total amount of these are tracked in case the wallet is used for reserves again in the future. In core.Core, the new wallet reserves methods are used to maintain adequate reserves based on the account's target tier and observed tier change. The dexAccount type has new fields to support this: - tierChange int64, is the total tier change that needs to be actuated with wallet reserves. Facilitates maintenance with changing tier. - totalReserved int64, helps track and debug the total amount reserved with the wallet's ReserveBondFunds method *only* for this dexAccount. Use of ReserveBondFunds and RegisterUnspent, facilitated by the new dexAccount fields, happens in: - connectWallets() via initialize(), after connectDEX(). Here it calls RegisterUnspent for all known bonds across all dexConnections. - authDEX. On reconnect, any observed tier change is acctuated with wallet reserves. Initial auth on login is handled slightly differently, (re)reserving the full required amount given the known unspent bonds. - handleTier change, which is called on penalization only, updates the tierChange field of the dexAccount. - rotateBonds. Any tierChange value is actuated with reserves. - PostBond. When making the initial bond for a new account, with maintenance enabled, ReserveBondFunds is used to pre-reseve. Here the boolean input is true, and the return is checked to ensure the wallet is sufficiently funded to increase reserves by the requested amount for the new bonds. - UpdateBondOptions. When modifying target tier or changing the bond asset. The boolean input and output are also used here. Also, remove the (*Core).connectAccount helper that should only be used on Core startup.
chappjc
force-pushed
the
bond-reserves
branch
from
2f403a7
to
76d5f64
Compare
|
Thanks to everyone who reviewed this PR. |
Requires #2036. In draft until that's settled.This PR is the commits starting with
client/{asset,core}: RefundBond broadcasts too.Commit messages (but there are a ton of "fixup" commits already pushed that will be squashed before this is merged):
client/{asset,core}: RefundBond broadcasts too
client/{asset,core,webserver}: BondLocked balance category
client/asset/dcr: respect reserves in funding
client/{asset,core}: reserves track as bonds are spend/refunded
client/{asset,core}: ReserveBondFunds and dexAccount.tierChange