round: verify and sign asset trees (M4 client) - #1107
Conversation
An asset VTXO request names its asset and amount; operators without a matching asset round reject it.
The join-auth TLV binds the asset identity and amount into the intent digest, appended only for asset requests so Bitcoin-only digests are unchanged.
The root binding recompute honors an asset tree's combined tweak, so the co-signed aggregate provably spends the committed batch output. Asset requests validate through ValidatePathForAsset, which pins the leaf's asset identity and exact amounts; the composed output script is proven at spend time with the leaf proof material. Signing sessions pick up per-node tweaks through the asset context lookup.
RegisterVTXORequestsRequest gains asset entries: each derives its owner key and standard policy like a plain request, then pins the asset identity and amount with a fixed carrier value. The waved test hook exposes it so integration harnesses can drive a client-requested asset round.
Under the seal-time fee handshake the designated change output is the residual sink: the server stamps it with what remains after the fixed outputs and fee at seal. Counting its full target against the input budget rejected any intent combining a change-bearing input with a fixed output, such as boarding plus an asset VTXO request.
The root's resolved input amount is zero (the proof verifier authenticates it at commit time), so re-registering it clobbered the subtree total on a single-leaf tree's only node and clients rejected the leaf with a zero asset amount.
1aa2174
into
darioAnongba/taproot-assets-oor-runtime
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 281aae695f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| if vtxoReq.AssetRef != "" { | ||
| clientTree, validateErr = | ||
| vtxoTree.ValidatePathForAsset( |
There was a problem hiding this comment.
Authenticate asset metadata before signing
For an asset request, this validation trusts asset_ref and asset_amount from the same operator-supplied tree that it is meant to verify. Because the wire conversion explicitly omits sealed packages and this path only requires the leaf output to be P2TR, an operator can send an ordinary Bitcoin tree, label its context with the requested asset and amount, and have it pass; the client then contributes tree signatures and can ultimately release boarding/forfeit signatures without receiving the requested asset. The asset commitment and amount need to be cryptographically authenticated against the leaf output before this security checkpoint succeeds.
AGENTS.md reference: round/AGENTS.md:L159-L163
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to extract client path: %w", err) | ||
| } | ||
| if err := clientTree.Verify(); err != nil { |
There was a problem hiding this comment.
Handle a missing asset signer path before dereferencing
When an operator-provided asset tree does not contain the requested signing key, ExtractPathForCoSigners returns (nil, nil). This immediately calls clientTree.Verify() and panics instead of rejecting the malformed tree, so a protocol peer can crash the round actor or daemon simply by omitting the client's signer from every asset tree.
Useful? React with 👍 / 👎.
| req.AssetRef = assetReq.AssetRef | ||
| req.AssetAmount = assetReq.AssetAmount | ||
| req.FixedAmount = true |
There was a problem hiding this comment.
Persist the composed script and asset metadata
Asset requests now enter the normal round flow, but the confirmation path still builds ClientVTXO with req.EffectivePkScript()—the untweaked policy script rather than the asset-composed leaf script—and ClientVTXO carries no asset reference, amount, or root. Consequently every successful asset round is persisted as a Bitcoin-only VTXO with a pkScript that does not match its actual outpoint, preventing correct asset discovery and subsequent spending.
Useful? React with 👍 / 👎.
| fixedOutput := totalOutput | ||
| if len(s.VTXOs)+len(s.Leaves) > 1 { | ||
| changeIdx := -1 | ||
| for i, vtxo := range s.VTXOs { |
There was a problem hiding this comment.
Subtract the actual designated leave change output
For a combined intent where a LeaveRequest is explicitly marked IsChange and there is also a non-fixed VTXO, this loop ignores the leave marker and subtracts the first non-fixed VTXO from fixedOutput. designateChangeMarker later preserves the explicit leave marker, so the preflight computes a different fixed-output set than the request it sends; depending on the relative targets, it either rejects a valid intent or admits one whose true fixed outputs exceed its inputs.
Useful? React with 👍 / 👎.
Client half of Milestone 4 (asset round trees), stacked on #1091. Operator counterpart: lumos#761 — the joint acceptance runs green against real tapd there.
What changes
VTXORequestgainsasset_ref/asset_amounton the wire and inlib/types; the join-auth TLV binds both into the intent digest (appended only for asset requests, so Bitcoin-only digests are unchanged).verifyVTXOTreeRoot) honors an asset tree's combined root tweak, proving the co-signed cosigner aggregate can spend the committed batch output; a tampered tweak is rejected.tree.ValidatePathForAsset: structural path validation and exact Bitcoin amounts as before, plus pinning of the leaf's asset identity and exact asset amount from the wire context. The composed leaf output script is deliberately not byte-verified here — that requires the leaf proof material which only arrives at spend time (Milestone 5).TweakLookupfrom the tree's asset context.RegisterVTXORequestsRequestaccepts asset entries (deriving owner keys and policies like plain requests) and a waved test hook exposes it for integration harnesses.