Let a wallet run a supervised asset's whole life cycle - #192
Open
albertodeluigi wants to merge 29 commits into
Open
Let a wallet run a supervised asset's whole life cycle#192albertodeluigi wants to merge 29 commits into
albertodeluigi wants to merge 29 commits into
Conversation
changeEvent() answers a palette change by re-colouring the four sidebar icons. The actions it names are created in createActions(), which the constructor reaches long after RestoreWindowGeometry() -- and restoring a geometry pumps events. A palette change that lands in between finds four null pointers and takes the process down inside QAction::setIcon, before the log file is open: an access violation with nothing written anywhere. On Windows this is not a rare race. The shell hands the theme to a process it activates, so launching from Explorer -- double-clicking the exe, which is how anyone actually starts a wallet -- crashed 5 times out of 5, while the same binary started from a shell survived 5 out of 5. The report is a 0xc0000005 at a fixed offset, five bytes into QAction::setIcon, which is the prologue dereferencing this. Guard each pointer. With the guard, 5 launches out of 5 from Explorer reach a window.
bitcoind runs one scantxoutset at a time for the whole process, and this file has three callers: the Bitcoin balance every loaded wallet refreshes on its own timer, a Bitcoin send, and the parent-chain history. Two wallets open in one GUI are enough to break it: their timers start together, so the two balance refreshes land in the same second, once a minute, for ever. The loser was handed bitcoind refusal and the GUI printed it verbatim -- "Scan already in progress, use action abort or status", advice addressed to nobody, parked under the balances until a refresh happened to find the slot free. On this machine it never did. The send side already waited for the slot. Share that wait with the other two, and make it a DEADLINE rather than a count of tries: a testnet4 scan measured 18.8 s over 14.2 million outputs, and a budget shorter than one scan means the second caller always gives up seconds before the slot frees. 90 s covers a scan several times that size and still bounds the wait, so an abandoned scan cannot hang a wallet. This is a mitigation, not the cure: two wallets still pay for two full scans a minute for a number that rarely moves. One scan per process, or a shared cached result, is the real fix and a larger change.
Every reading of a parent-chain balance costs a full scantxoutset: measured 18.8 s over 14.2 million outputs on testnet4, and a mainnet-sized UTXO set is over an order of magnitude larger -- minutes per reading, on a refresh that ran every 60 seconds per open wallet. Most of that work confirmed that nothing had changed. The node already walks every new parent block, output by output, looking for PoS checkpoints. Hand it the scripts and outpoints a wallet cares about and the same walk answers a second question for free: did this block pay, or spend, anything of ours. The count rides to the GUI inside getanchorstatus, which the overview polls every tick anyway, so knowing costs no RPC at all. - anchor: a watch list (raw scripts and outpoints) and a monotonic touch count, checked during the existing block walk. The output test sits BEFORE the OP_RETURN filter -- a payment to us is an ordinary output -- and the walk now also reads vin, because a spent coin moves a balance as much as a received one and is invisible otherwise. - wallet: getbtcbalance registers what to watch after a scan that succeeded. A failed scan knows nothing and must not replace what was known. - qt: refresh when the count moves, at most once a minute, plus one scan every ~30 minutes regardless -- for a node not walking blocks, a wallet with more UTXOs than the list holds, and the cold start before the first scan. - rpc: getbtcscanprogress, so the first scan of a wallet can show how far it has got instead of an empty line for minutes. Steady state goes from one full scan a minute per wallet to none.
The wallet held no record of its parent-chain coins. Every balance, and every send, rebuilt the answer from nothing with a scantxoutset over the entire parent UTXO set: 18.8 s across 14.2 million outputs on testnet4, minutes on a mainnet-sized set. That single fact caused everything reported today -- a balance that took twenty seconds, a Send window frozen twice while it asked the same question again, and a second send refused by Bitcoin as a replacement of the first, because a scan sees CONFIRMED coins only and so offered the coin the first send had already committed. Every serious wallet keeps what it owns and edits it as blocks arrive. Sequentia assets have always worked that way, which is exactly why they were always instant. This gives Bitcoin the same treatment. - parent_coins.json beside the wallet: the coins, the height and hash they were current at, and any pending spend of ours. The hash is what makes a parent reorg visible instead of silently describing a chain nobody is on. - Filled by one full scan, then advanced block by block, in both directions: a spent coin moves a balance as much as a received one. More than 500 blocks behind, or across a reorg, one full scan is the faster answer, and the code says so instead of guessing. - getbtcbalance answers from the record: measured 33.6 s -> 80 ms -> 55 ms on the same wallet, same coins, same totals to the satoshi. - A send picks from the record, marks what it committed, and adds its change at height 0, so the next send neither collides with the last nor waits for a confirmation. - getbtcfeerate, and a Bitcoin fee panel in Send. The asset fee panel is hidden for bitcoin because it prices assets, which left no fee controls at all: Recommended now shows the number -- testnet4 quoted 362 sat/vB, which is 0.0005 BTC of fee on a 0.01 BTC send -- and Custom lets it be refused. - Bitcoin DEFAULT_TRANSACTION_MAXFEE ceiling, for bitcoin sends only. Sequentia dropped the default ceiling because fees there may be paid in any asset; this fee is bitcoin, and the estimator behind it can be absurd.
Every parent RPC opens its own TCP connection and authenticates again, so the cost of catching up is counted in round trips, not in blocks. Asking for a height whose hash the previous block already told us is a whole connection spent on nothing: 500 blocks behind meant 1000 of them. Follow the nextblockhash a block already carries. Same blocks, same order, half the connections -- and the half that remains is the half that returns data we do not already have. Upstream 24.7.5 adds CallMainChainRPCBatch, which is the real answer for the catch-up path; this stands on its own until we rebase onto it.
With Bitcoin Core stopped, getbtcbalance answered 0.00000000. The record beside the wallet held three coins at the time; the wallet reported none of them, because the daemon it wanted to ask was not there. A zero balance reads as theft. A dated one reads as weather. When the parent chain cannot be reached, answer from the record and mark the answer stale: the coins existed at parent_height and nothing has been seen since. The overview says which block the figures are from and that Bitcoin is not answering, instead of replacing them with a zero. Falling through to a full scan here was pointless anyway -- it needs the same daemon. This is the other half of what a local record is for. It is not only faster than asking; it is what lets a wallet keep working when there is nobody to ask.
The catch-up limit was 500 blocks, chosen before anything had been measured. Then Bitcoin Core was stopped for a while and the wallet had 38 blocks to apply: it took 33.5 s, about 0.9 s a block. A full scan of the same wallet takes 33.6 s. So the crossing point was around forty blocks, and the limit as written would have spent seven and a half minutes catching up to avoid half a minute of scanning. The cost per block is the connection, not the block -- every parent RPC opens a fresh TCP connection and authenticates again. The cost of a scan is the size of somebody else s UTXO set. Neither is a constant this code can know, and they move in different directions between testnet4 and mainnet. So measure one and compare: the record now keeps how long its own last full scan took, and catching up is chosen only while it is cheaper. Batched parent RPC (upstream 24.7.5) will cut the per-block side sharply; this arithmetic follows it without being edited.
Three faults with one root: a send that has been broadcast but not mined is a state this code kept failing to represent. - A rebuild of the coin record dropped it. A scan sees the CONFIRMED chain, so it re-offered the coin an unconfirmed send had already committed and knew nothing of that send change. Any rebuild -- migration, reorg, realignment -- silently re-armed the double spend the record exists to prevent. A rebuild now carries both across. - Its confirmations read as unknown. They were inferred by asking whether an output of the transaction still existed; once a later send spent those outputs the answer said nothing, so a transaction with 42 confirmations was reported as unknown minutes after its change was spent. The record holds the height of every coin it has seen; ask it. - It looked broken. Zero confirmations drew transaction_0, the question mark that means nobody can vouch for this state, where the clock was meant. The question mark now appears only when the count really cannot be established. Also: a send rings the watch bell itself. It happens in a mempool, no block walk can see it, and the overview would have shown the spent coin until the next block or the half-hour safety refresh.
Typing an amount in tBTC put an error dialog on screen: No asset provided for recipient. Nothing to do with sending -- the send was never reached. refreshTxSize runs 400 ms after the last keystroke and asks the wallet to build the transaction, because the only honest size is the one it would really build. For a Bitcoin recipient that transaction cannot exist: bitcoin does not travel in a Sequentia transaction, its asset is the null one by design, and the wallet answers exactly that. The probe treats a failure as no total and moves on -- but WalletModel::prepareTransaction shows the wallet error to the user before returning, so the failure was not silent, and every keystroke produced a modal error for anyone typing an amount in bitcoin. Skip Bitcoin recipients in the probe, as the real send already does by taking the parent-chain road. The dialog was a nuisance rather than a blocker, which is why a send still went through after dismissing it.
A functional test for the record behind the Bitcoin balance: it is built on first use, moved forward over new blocks without sweeping the UTXO set again, sees money arrive, survives a restart, is rebuilt rather than walked across a parent reorg, and answers from what it knows -- marked stale -- when the parent chain cannot be reached. Writing it found two real faults: - The balance was read from a field whose NAME depends on which daemon answers: bitcoind says total_amount, an Elements-style daemon says total_unblinded_bitcoin_amount. Against the latter the wallet reported zero next to a list of coins plainly present. The total is now summed from the coins being recorded, so the balance and its own coins cannot disagree, and the field name stops mattering. - Short gaps were rescanned rather than walked whenever a scan happened to be quicker, which on a small chain is always. But walking is not merely faster: a scan sees only the confirmed chain and drops what only the record knows -- coins committed to a send still in a mempool, and its change. A wallet that rescans because rescanning is cheap would keep forgetting its own pending sends. Gaps under twenty blocks are now walked whatever the clock says. Not covered here, and said so in the file: sending. sendbtctoaddress builds a Bitcoin transaction, and the daemon standing in for Bitcoin in this framework is an Elements one that cannot decode it. Those three properties were exercised by hand on testnet4 instead.
The Send form has an Add Recipient button, and for bitcoin it led to a refusal:
one recipient, no other assets. Half of that was a real constraint and half was
ours. Bitcoin and a Sequentia asset genuinely cannot share a transaction -- two
chains -- but several Bitcoin recipients are an ordinary Bitcoin transaction
with several outputs, and sending them together costs one fee and one set of
inputs instead of one of each.
sendbtctoaddress now takes either an address and an amount, as before, or a
list of {address, amount}. The change output moves to the end, after the
recipients, and the record follows it there: getting that index wrong would
have lost track of our own change again.
subtractfeefromamount stays limited to a single recipient. With one the
deduction is unambiguous; with several it is a decision about whose payment
shrinks, and the wallet should not make it quietly.
The GUI says it earlier, too. Mixing chains now shows a warning in the form
while it is being filled, rather than waiting for Send to refuse a form the
user has already finished typing -- and the refusal, when it comes, says the
true reason instead of the old message that blamed the recipient count.
The Sequentia path already handled several recipients; nothing there changed.
The multi-recipient send recorded one address and one total, so the history showed a single row of -0.15 next to the first destination and the other two payments were simply gone. The parent chain cannot give them back either: once the outputs are spent there is nothing left to scan for. Record every destination, and emit one row per destination when reading the history back. The fee belongs to the transaction rather than to any one payment, so it rides on the first row only -- otherwise anyone adding up the column counts it once per recipient. Sends recorded before this keep their single row: the information was never written down, and inventing it would be worse than the gap.
Three costs grew with the number of sends ever made, and one of them grew with its square. - Reading the history re-parsed the entire coin record INSIDE the loop, once for every send whose outputs were already spent -- which, given time, is nearly all of them. A thousand sends meant a thousand reads of one file. Read it once. - Every send was re-checked against the parent chain on every read: up to two calls each, for ever, including transactions buried months ago. Six confirmations is final, so note it and stop asking. The marks are written once per read rather than once per send, or the saving would have moved from the network to the disk. - Rebuilding the record asked the parent chain about every send ever made. Settled ones have nothing left to say. And one that was not about scale at all: a send took up to a minute to appear. The overview re-reads the balance at most once a minute -- a limit that exists so a failing read cannot become a retry storm -- but it was applied to events too, including the users own send. Bitcoin Core shows the same transaction instantly because it holds it. Now a move we have been told about is served at the next tick, and the minute only rate-limits the failures it was written for.
… holds The scenario fabricates 503 recorded sends -- 500 buried, 3 still waiting -- because a wallet with a year behind it is not something a test can produce one transaction at a time, and counts the calls the read costs with getmainchainrpcstats rather than timing it. It found the previous commit to be half a fix. Marking settled sends and skipping them when rebuilding the record saved nothing on the path that mattered: reading the history still asked the parent chain about every send, 503 calls for 503 sends. Two things were missing. - The calls had to stop for settled sends, not just the marking. - To stop asking, the confirmations must be computable: hence the height a send settled at, recorded when it is marked. And a case the test found on its own: a send marked settled WITHOUT a height -- which is what every wallet upgrading from the previous build has -- fell through to asking again. Buried is buried either way. 503 sends now read with 3 calls.
The Bitcoin fee panel was written against a Send dialog that built its fee controls out of a shared FeeSelectionWidget. This base solves the same problem differently -- its own grid, no such widget -- so the member that held it, and the comment describing it, arrived here with nothing behind them, and the header stopped compiling. The panel itself needs none of that: it attaches to ui->frameFee, which this base has, at the point where that frame is hidden because every recipient is being paid in bitcoin.
The rebase onto v24.7.7 brought in a timer this base already had. Both blocks ran in the constructor: the first was allocated and connected, then the second overwrote the pointer, leaving a timer that is connected to refreshTxSize and can never be started -- and leaving the reader to wonder which of the two is the live one. Nothing was visibly wrong, which is the reason to fix it now rather than when someone changes one of the two and cannot work out why it had no effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Left behind by an assertion that changed shape while the test was being written. flake8 F401, and the lint job is right to stop on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… asset Supervision records were funded by hand: the page picked an outpoint itself and charged a fixed 100000-atom fee. With the open fee market there is no asset to default that fee to, and no reason for a page to price a transaction the wallet can price properly. The build order is inverted so the wallet can do it. buildsupervisionrecord is called with a placeholder signature of 64 zero bytes -- it does not verify signatures, and the placeholder is exactly as wide as the real one, so the transaction priced in the probe pass is the transaction that gets broadcast. That record is funded with an explicit fee_asset, vin[0] is read back, getsupervisionrecordhash runs over that outpoint, the issuer signs, and the record is funded again with vin[0] preset. Elements' FundTransaction keeps preselected inputs and only appends, but vin[0] is checked before signing anyway and the broadcast is refused if it moved. An unfreeze needs no probe pass -- its sighash binds the record outpoint, not a funding input -- so it signs first and funds once. Two wallet-layer faults this uncovered: - A preselected input the wallet cannot sign for was dropped from the fee estimate while staying in the transaction. That is the freeze record exactly, spent by consensus rule rather than by a key, and the unfreeze came out about 106 vbytes short -- enough to miss the relay floor. input_weights now wins before the loop gives up, and SUPERVISION_UNFREEZE_INPUT_WEIGHT states the width. - Preset-input accounting charged an input's weight against its own asset's target instead of the fee asset's. For a same-asset input the two new lines collapse into the one they replace. fundrawtransaction's "TX must have at least one output" now also accepts a transaction that has inputs: an unfreeze spends the record, pays a fee, and has nothing else to say. FeeSelectionWidget owns the selector, the four-cell grid and the three-tier warning, and both pages use it. The Send tab hands over its Recommended/Custom block through setRateModeWidget(), so the visual order is unchanged and neither page depends on the other. It re-reads on assetTypesChanged, balanceChanged and numBlocksChanged. Records are marked non-replaceable: bumping a fee re-runs coin selection, and the admission signature covers the input it chose, so RBF would offer a repair that destroys the thing it repairs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Carried onto the parent-chain wallet work: buildFeeGrid() moves into the widget and it was where the Bitcoin fee controls and the mixed-chain warning had been hooked, so those move to the constructor, which is where they sit in the branch the two halves grew up in together.
…ing the funds A send of a supervised asset to a frozen script is refused by consensus: testmempoolaccept answers bad-txns-asset-frozen, and the transaction never enters any mempool while the freeze stands. The wallet showed it as an ordinary unconfirmed payment with a "?", indistinguishable from one still being relayed, and counted its inputs as spent -- so both the asset and the asset paying the fee left the Overview balances entirely, not even under untrusted_pending. From the holder's side, funds vanished with no explanation. Refusals are not abandoned automatically, and must not be: lift the freeze and the same transaction becomes valid. A transaction missing from our own mempool may equally have been evicted, or never relayed -- Bitcoin Core's caution here is well founded. So the node is asked instead. Chain::checkMempoolAccept is testmempoolaccept for a single transaction, returning the refusal and whether it was consensus rather than policy. It answers "cannot say" for missing inputs, which is what a child of an unconfirmed parent looks like, and for a transaction already in the mempool or already mined. CWalletTx carries the answer in memory only, never serialised: a refusal is a fact about the chain now, not about the transaction, so the wallet re-asks rather than ever believing a stale no -- and the wallet file stays readable by a build that knows nothing of any of this. A sweep on the wallet scheduler re-derives it every 20 seconds, skipping coinbase, abandoned, confirmed and in-mempool transactions, so it costs nothing on a quiet wallet; any submit attempt resets the clock, so a refused send says so at once. IsSpent treats a consensus-refused transaction's inputs the way it treats an abandoned one's, and the funds return to the balance by themselves. Only consensus: a policy or fee refusal leaves them locked, because such a transaction can still be mined elsewhere. The record stays visible and is not abandoned, so the inputs lock again on their own the moment the node stops refusing. The GUI gains TransactionStatus::Rejected beside Abandoned, drawn with the transaction_denied icon and the danger colour -- the same definitive "no" already used for an orphaned block reward, rather than a "?" that promises a state which may still resolve. The reason is rendered as a sentence, and the details dialog carries the Abandon action: nobody goes looking for a context menu on a row that claims to be merely pending. The functional test covers the two opposite cases on the supervised-assets fixture: a spend the freeze refuses is reported and gives its funds back, and a valid transaction that simply was never relayed is left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It shared :/icons/assets with the Assets tab, which made two different sections look like one thing. The padlock says what this tab is for: freeze, unfreeze, pause and rotate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The selector was filled once, when the wallet was attached, and the asset registry is fetched over the network and merged well after that. An asset whose label resolved later kept the 64-hex id it was given at build time for the life of the window, so GOLD and tADLT read as raw ids in the one field whose whole job is to say which asset pays. Not new, and not the widget's doing -- the code it was lifted from called assetDisplayName() at the same single moment. It was simply the last long- lived selector without the correction the others already have: TransactionView re-labels its asset filter on a timer, AssetsPage repopulates on show, and SendCoinsEntry has refreshAssetNames(). This adds the same to the widget, where both pages inherit it: on show, and on every refresh, which is already wired to numBlocksChanged. Only the item text is rewritten -- items, asset ids and the current choice stay as they are -- and only when it actually differs, since setItemText on the current item makes the combo emit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moving the fee grid into FeeSelectionWidget deleted the block of setup code that built it -- and took with it the four adjacent lines that created and connected m_size_timer, which had nothing to do with the grid and only happened to sit at the end of the same function. m_size_timer defaults to nullptr, so the guarded start in coinControlUpdateLabels() silently did nothing, refreshTxSize() was never reached, and m_tx_vsize stayed zero for the life of the window. On screen that is a "Total for this transaction" showing an em dash and refusing to be typed into even under Custom, because the total cells are enabled only when there is a size to divide by -- so the one cell a user is most likely to want to set the fee from was dead. Restored beside the widget's construction, which is the setup that happens once, rather than in updatePreferredFeeAsset(), where the deletion left its replacement line and which runs on every keystroke. Found by using it. The rest of that commit's removals were checked against their replacements and are accounted for: payAmountChanged reaches updatePreferredFeeAsset, optInRBF reaches setReplaceable, and assetTypesChanged and balanceChanged are wired inside the widget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The page argues, in its own words, that "a record that waits is a record the target can outrun -- so it is worth paying for the next block rather than the cheapest one", and then offered no way to do it: the fee panel showed the recommendation and nothing else. Only the Send tab had Recommended/Custom, because only the Send tab had the controls to hand over with setRateModeWidget(), and a host that supplies none was left on the recommendation for good. FeeSelectionWidget now builds its own Recommended/Custom pair when the host supplies none, and drops it the moment one arrives, so the Send tab is untouched and any later page gets the choice for free. Switching to Custom seeds the rate from the figure being quoted -- through the same conversion onCellEdited() uses, or moving the radio would move the fee by itself -- so the user starts from the recommendation and raises it, which is the whole point here. Nothing was needed on the funding side: fundRecordTransaction() already reads hasCustomRate() and passes fee_rate to fundrawtransaction. The control was the only missing part. Two things this exposed. The total cell was dead on this page, because the total can only be typed into when there is a size to divide by and SupervisionPage supplies a fee outright rather than a size. It has one -- it funds a placeholder record to price it -- so it now reports the vsize of that funded transaction as well. And a quoted total must not survive into Custom. updateGrid() preferred m_known_total whenever it had one, which is right under Recommended, where it is the fee the wallet actually charged. Under Custom it would have pinned the total to the recommendation while the user raised the rate underneath it -- the panel quoting one fee and the wallet charging another, which is exactly the class of fault the fee panel has already been fixed for four times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The history called a transaction in the last block "Confirming (1 of 2 recommended confirmations)", which reads as unfinished business, and as an instruction to keep waiting for something. There is nothing further to wait for that a second block supplies. RecommendedNumConfirmations becomes 1, so a transaction is either unconfirmed or confirmed and the intermediate state no longer arises. Display only: it moves no wallet policy, nothing about when a coin becomes spendable, and no consensus rule. The Confirming branch and its icons are left in place, correct again the moment anyone raises the constant. "Confirmed (%1 confirmations)" becomes the %n plural form, since with one confirmation now the ordinary confirmed state, "Confirmed (1 confirmations)" would have been the string most often on screen. Thirteen other strings in this GUI already use that form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…able Two wrong sentences on the same screen, both found by pausing an asset and watching what the wallet said about the refusal. THE REASON NAMED THE WRONG RULE. A pause is stored as a freeze on every target -- SupervisionRegistry::IsFrozen answers for any target once SUPERVISION_PAUSE_TARGET is present -- so consensus refuses a paused asset with bad-txns-asset-frozen. Right as a rule and wrong as an explanation: the holder is told their address was singled out and goes looking for a freeze that was never placed. IsPaused() already exists for exactly this, its comment saying it is "distinct from IsFrozen only for reporting: an RPC has to be able to say WHY a spend is blocked" -- it simply was not consulted on this path. checkMempoolAccept now asks it, taking the asset from the debug message consensus already writes there, and reports bad-txns-asset-paused. Drawn in the node, where the registry is, so it is decided once per transaction rather than per row, and so the wallet needs to know nothing about what a pause is. The consensus reject reason is untouched; this string is returned through that interface and never goes on the wire. AND "AVAILABLE AGAIN" WAS TRUE OF HALF THE MONEY. Releasing the inputs of a refused transaction puts every asset it held back in the balance, and for the asset paying the fee that really does mean spendable again. For the frozen or paused asset it does not: the units are back in the totals and cannot be moved by anyone, anywhere, which is the whole point of the freeze. On the reported transaction that was 2.99999981 GOLD genuinely free and 3.997 of a paused asset that was not, both announced as available. The claim that mattered was only ever that nothing went missing, so the short form says "back in your balance" and no longer promises spendability. The details add the split -- fee spendable, asset still blocked, wherever it is held -- for the two refusals where the asset itself is what is refused. A supervised asset sent to a confidential address, or a malformed record, refuses that transaction and leaves the holder's units alone; those keep the plain wording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lifting a supervision freeze -- a pause included, which is a freeze on every address -- spends the freeze record, pays a fee, and has nothing else to say. It has no recipient because there is nothing to send. The fundrawtransaction guard was already relaxed for this, with a comment saying so. The relaxation stopped one gate short: FundTransaction builds its recipient list from the transaction's outputs, an unfreeze has none, and CreateTransaction refuses an empty list a few frames later. So the RPC let the transaction in and the wallet turned it away, with "Transaction must have at least one recipient" -- a message about a mistake the caller had not made. Lifting a pause could not be funded at all. Same relaxation, same reasoning, on the second gate: an empty recipient list is still refused, unless the caller has named the inputs to spend. A caller that forgot where the money goes has selected nothing and is still caught; one that has selected inputs is asking to spend THEM, and the change and fee that funding appends are the only outputs it needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Change is per asset, and the reserve-destination vector is sized by walking the recipients: one for the policy asset, one more per distinct recipient asset. The loop that hands out change scripts then serves the preselected inputs from that same vector -- and those were never counted. An asset that arrives on a preselected input and is named by no recipient runs off the end. Running off the end shares its branch with GetReservedDestination() failing, so it reports "Keypool ran out, please call keypoolrefill first". The one caller that hits it is told to refill a keypool that is full, which is where this was found: a wallet with 1000 keys in each pool, refusing to build a transaction for want of a key. Reachable today by anything that preselects a coin of one asset and pays someone in another -- coin control, or fundrawtransaction with a chosen input. It is unconditional for lifting a supervision freeze, which spends the freeze record and pays nobody at all, so the record's asset is named by no recipient by construction. Fixed where the sizing happens, with the same assets_seen set the recipient loop uses, so an asset already covered by a recipient does not reserve twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two faults in the test I added this morning, both of which only a run could find, and it had never been run. It read the policy asset out of getbalances() by hex id. That map keys an asset by its LABEL where it has one, so the policy asset arrives as bitcoin and the hex lookup raised KeyError. An asset with nothing spendable is also absent from the map rather than present as zero, so both reads now go through a helper that tries the label, then the id, then zero. And the control case assumed the holder had a coin to pay with. By that point everything it holds is change of the refused family: released back into the balance, correctly, but unconfirmed and so not trusted, which left coin selection nothing to spend. It is now given a confirmed coin first, with an assertion saying so, since a control that cannot be built would otherwise pass by doing nothing.
The control case ended by restoring the holder and calling sync_all(), which waits for the mempools to match as well as the tips. That is the one thing the scenario cannot promise: the node had spent the previous minute deliberately not relaying, so the sync timed out at sixty seconds and failed a test whose assertions had all already passed. Blocks are what the tests after it need. It also left the unrelayed transaction in the wallet. Restoring the node allows broadcasting again, so it would have gone out immediately and every later test would have run against a mempool holding a transaction that exists only to make a point about relay. It is abandoned before the restart.
albertodeluigi
force-pushed
the
supervised-assets-wallet
branch
from
August 26, 2026 22:58
b5a3324 to
79ef231
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let a wallet run a supervised asset's whole life cycle
What could not be done at all
Freezing a holder works today. Unfreezing does not — not from any wallet,
not from the GUI, not from
fundrawtransaction. A pause is a freeze on everyaddress, so resuming a paused asset is the same wall.
Lifting a freeze is a transaction with nothing to say: it spends the freeze
record, pays a fee, and has no recipient, because there is nobody to pay.
Three separate things refused to build it, each hidden behind the one before:
fundrawtransactionrejected it outright — "TX must have at least oneoutput". With neither an output nor an input there is indeed nothing to
fund; a transaction that already names its inputs is a different case,
because spending them is the point.
leaving it in the transaction. Sizing an input means producing a dummy
signature, and a freeze record is deliberately not spendable by any key —
consensus admits it. Sizing returned -1, the input was skipped, and the fee
came out ~106 vbytes short: enough to miss the relay floor.
this transaction has none — so the record's asset is named by nobody. Running
off the end of that vector shares a branch with an empty keypool, so the one
caller that hits it is told "Keypool ran out, please call keypoolrefill
first" on a keypool that is full.
The three are a chain: removing one just moves the error. All three are here.
A refused transaction said nothing and held the funds
A spend the freeze invalidates is evicted from the mempool. The wallet is then
in a state it cannot read: a transaction it made, unconfirmed, in nobody's
mempool, not abandoned. Consensus will refuse it for as long as the freeze
stands — but nothing said so, and the inputs it reserved were simply gone from
the balance, the fee asset included.
The opposite case looks identical from the wallet's side: a perfectly valid
transaction that has not been relayed yet. That one must be left exactly as
Bitcoin Core leaves it. The node is asked, on a timer, which of the two this is;
only a consensus refusal releases the inputs, and it is reported rather than
silently repaired. Nothing is abandoned on the user's behalf: lifting the freeze
must be able to bring the transaction back.
Also: a pause was reported as a freeze, and frozen funds were counted as
available.
Tests
feature_supervised_lifecycle_wallet.py(new): the whole life cycle throughthe wallet — issue, distribute, freeze, refuse, unfreeze, pause, refuse,
resume, rotate, and the old key no longer signing. Every step is the
wallet's own RPCs, so a GUI can do it. Gaps are collected rather than fatal,
so one run reports all of them.
feature_supervised_assets.py: two cases added — a refused spend is reportedand frees its funds; a merely unrelayed one is left strictly alone.
Measured on v24.7.7 (
0aa3fbb15b): the life cycle reports two gaps, and therefusal case fails with the holder's balance never coming back. With this branch
both are green, as are
rpc_fundrawtransaction,wallet_basic,wallet_send,wallet_fundrawtransactionandwallet_txn_clone.The Supervision tab prices its own fee
FeeSelectionWidgetis introduced here and used by both pages: the Send tabhands it its Recommended/Custom block so the visual order is unchanged, and the
Supervision tab gets a fee selector it never had. Records were funded by hand
before this — the page picked an outpoint itself and charged a flat 100000
atoms, which under an open fee market is a number with no asset attached.
Records are marked non-replaceable on purpose: bumping a fee re-runs coin
selection, and the admission signature covers the input that selection chose, so
RBF would offer a repair that destroys the thing it repairs.