Skip to content

feat(burn)!: write burn claim proofs with a burn output proof - #145

Merged
brianp merged 4 commits into
mainfrom
feat/burn-output-proof
Sep 28, 2026
Merged

brianp merged 4 commits into
mainfrom
feat/burn-output-proof

Conversation

@sdbondi

@sdbondi sdbondi commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Description

Adopts the burn claim proof shape from tari-project/tari#8054: the proof shows the burn output was mined (against block_output_mr) instead of the kernel.

  • BurnProofWorker fetches /generate_burn_output_proof?commitment=<hex> instead of /generate_kernel_merkle_proof.
  • The worker rejects a proof whose output preimage does not hash to the stored burn output hash, and retries next cycle.
  • The local CompleteClaimBurnProof/BurnClaimProof/AbridgedTransactionKernel mirrors are replaced by the tari_sidechain types.
  • mined_in_epoch (now required) is computed from the proof's block height using the network's consensus constants. BurnProofWorker::new now takes the Network.
  • Stale "kernel merkle proof" doc comments and openapi.json are updated.

The tari crates are bumped to 6.0.1-pre.1, which includes tari-project/tari#8054.

The burn_proofs table still stores the kernel fields and sender offset key. They are no longer used to build the proof file but are left in place to avoid a migration.

Motivation and Context

Lets Ootle bind a burn claim to the burn output's features (sidechain_id, claim_public_key), which the kernel cannot carry.

How Has This Been Tested?

  • cargo test -p minotari burn passes, including updated assemble_complete_proof tests and a JSON round-trip of the complete proof.
  • cargo clippy -p minotari --all-targets is clean.
  • Not tested against a live base node.

Breaking Changes

  • The burn proof file format changes: output_proof replaces commitment, encoded_merkle_proof, kernel and sender_offset_public_key, and mined_in_epoch is required.
  • Proof files written by this build can't be claimed until the consumers parse the new shape: Universe (ootle/claim.rs) and tari-ootle (MinotariBurnClaimProof) must ship the new tari_sidechain type first or together.
  • BurnProofWorker::new takes a Network, which breaks Universe's call site at compile time.
  • Needs a base node that serves /generate_burn_output_proof. A pruned node can only prove burns within its pruning horizon.

Adopt the burn claim proof shape from tari-project/tari#8054, which proves the
burn output was mined (against the block's `block_output_mr`) instead of the
burn kernel.

- Fetch `/generate_burn_output_proof?commitment=` instead of
  `/generate_kernel_merkle_proof`.
- Reject a proof whose output does not hash to the expected burn output hash.
- Use `tari_sidechain::{BurnClaimProof, CompleteClaimBurnProof}` instead of
  local mirrors, and fill the now-required `mined_in_epoch` from the proof's
  block height.
- Point the tari crates at `../tari` path deps until the change is released.

BREAKING CHANGE: the burn proof file format changes and needs a base node
that serves `/generate_burn_output_proof`.

@sdbondi sdbondi left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Posted by Claude.

Comment only — 0 blocking, 2 non-blocking, 1 nit · head 1b77a51

Own PR, and CI is red only because the ../tari path deps don't exist on the runner (cargo metadata fails to load the manifest), so no approve click. Path deps not raised, per the draft note.

Checked against tari at 3792c5008: BurnClaimProof/CompleteClaimBurnProof field shapes, GenerateBurnOutputProofResponse { proof }, the node handler's hex commitment query param, and OutputHashPreimageExt::hash (documented as matching TransactionOutput::hash, so comparing against the scanner-stored output_hash is sound). mined_in_epoch is computed identically to the console wallet's transaction_event_handler.rs (consensus_constants(height).block_height_to_epoch(height)). Locally cargo test -p minotari burn (12 pass) and clippy on minotari are clean.

Non-blocking

  1. The output-hash guard is untested — minotari/src/tasks/burn_proof_worker.rs:115. It's the one new piece of logic in the worker, and nothing exercises either branch; the tests only cover assemble_complete_proof. Pulling the check into a small pure fn (fn check_output_proof(expected: &FixedHash, proof: &BurnOutputProof) -> Result<()>) would make both the match and the mismatch path a two-line test each. The test fixture already builds an OutputHashPreimage from TransactionOutput::default(), so the expected hash is one .hash() away.
  2. Pruned-node 410 GONE retries forever at warn — burn_proof_worker.rs:95. The description notes a pruned node can only prove burns within its horizon; once a burn is outside it, the worker logs the same warning every 60s indefinitely and the proof never completes. Not a regression in kind (the kernel endpoint had the same failure mode), but the new endpoint gives a distinct status for it — worth either surfacing it distinctly ("use an archival node") or at least noting it in the burn_proofs_dir / burn-funds docs.

Nits

  1. DbBurnProof still carries kernel_excess*, sender_offset_public_key, kernel_fee, kernel_lock_height, and get_pending_burn_proofs still selects them, though nothing reads them now. Fine to keep the columns to avoid a migration as stated, but a one-line comment on the struct saying they're vestigial would stop the next reader wondering what uses them. The pending_merkle status name is similarly stale.

Sound

  • Removal of the local mirror structs in favour of tari_sidechain types; the JSON round-trip test covers the new required mined_in_epoch.
  • Rejecting a mismatched proof and retrying rather than writing a useless file.
  • BurnProofWorker::new building one ConsensusManager up front rather than per proof.
  • Doc/openapi wording updates are consistent across all touched sites.

- Move the output hash check into check_output_proof and test both branches.
- Log a 410 GONE from the base node as an error that points at an archival
  node, instead of the generic retry warning.
- Note the vestigial burn_proofs fields and the stale pending_merkle status
  name, and document the pruning limit on burn_proofs_dir.
@sdbondi

sdbondi commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Posted by Claude.

Addressed in 2079d7b:

  • Output-hash guard untested: moved into check_output_proof, with tests for match and mismatch.
  • Pruned-node 410: is_pruned_block_error detects 410 GONE and logs an error that points at an archival node. It still retries each cycle in case the node is switched. The burn_proofs_dir docs now mention the pruning horizon.
  • Nit: added doc comments marking the kernel_*/sender_offset_public_key fields as vestigial and explaining the pending_merkle name.

cargo test -p minotari burn (15 pass) and clippy are clean.

@sdbondi sdbondi left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Posted by Claude.

Comment only — 0 blocking, 0 non-blocking, 0 nits · 3 resolved since 1b77a51 (2 non-blocking, 1 nit) · head 2079d7b

Own PR, and CI is still red only on the ../tari path deps, so this is a comment rather than an approval. On the code, nothing stands.

Resolved

  • Output-hash guard untested: now check_output_proof (burn_proof_worker.rs:162), with a match test and a mismatch test. The matching test derives the expected hash from the fixture's preimage, so it exercises the real OutputHashPreimageExt::hash path.
  • Pruned-node 410 retries silently at warn: is_pruned_block_error downcasts to HttpError::ServerError { status: GONE }. Verified this works: send_request returns HttpError for any non-2xx, and get_burn_output_proof converts it with a bare ?, so the downcast target is not wrapped in context. reqwest-retry's default policy does not retry 410, so the error reaches the worker straight away. It logs at error! with an "use an archival node" hint, and the burn_proofs_dir doc now carries the same caveat. The test covers GONE, NOT_FOUND and a non-HTTP error.
  • Vestigial DB fields / pending_merkle name: now documented on DbBurnProof and the status constant.

cargo test -p minotari burn_proof_worker (9 pass) and clippy on minotari are clean locally at this head.

@sdbondi
sdbondi marked this pull request as ready for review September 28, 2026 13:46

@brianp brianp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed against tari#8054 and the published 6.0.1-pre.1 crates. The contract lines up: hex `commitment` query param, `BlockBodyPruned -> 410 GONE`, and the `mined_in_epoch` computation is identical to the console wallet's. `cargo test -p minotari burn` (15 passed) and `cargo clippy -p minotari --all-targets` are clean locally, CI green.

Non-blocking observations, none introduced by this diff:

  • Downstream shape mismatch: Universe's `ootle/claim.rs` and tari-ootle's `MinotariBurnClaimProof` still parse the old shape (`commitment`/`encoded_merkle_proof`/`kernel`). Until both ship the new type, a proof file written by this build is not claimable. Worth a line in Breaking Changes so the bump order is explicit. `BurnProofWorker::new` gaining `Network` also breaks Universe's call site (compile-time, easy).
  • Pruned branch log: the new `error!` arm drops the `error:% = e` field the `warn!` arm has, so the node's 410 body never gets logged on the one path where it is most useful. Also re-fetches every 60s forever with no backoff; a `pruned` status would stop that, but fine to defer.
  • Epoch calc untested: the only new arithmetic (`consensus_constants(height).block_height_to_epoch(height)`) sits behind the HTTP call and every test threads a literal epoch in. Passing the `ConsensusManager` into `assemble_complete_proof` would let one LocalNet test pin it.
  • Pre-existing and out of scope: no confirmation-depth gate before finalising (reorg leaves a stale proof + epoch), `block_hash`/`block_height` from the node are not cross-checked against the wallet's own scanned output row, and `write_proof_file` is not atomic relative to `mark_burn_proof_complete`.

🤖 Generated with Claude Code

@sdbondi

sdbondi commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Posted by Claude.

Thanks @brianp. Addressed in 556ff28:

  • Downstream shape mismatch: added the Universe/tari-ootle bump order and the BurnProofWorker::new signature change to Breaking Changes.
  • Pruned branch log: the pruned-block error! now includes error:% = e, so the node's 410 body is logged. I left the pruned status/backoff for later.
  • Epoch calc untested: moved it into height_to_epoch, with a LocalNet test at epoch boundaries.

I didn't change the out-of-scope items (confirmation depth, block hash cross-check, atomic write).

@brianp
brianp merged commit f3f6c9e into main Sep 28, 2026
2 checks passed
@brianp
brianp deleted the feat/burn-output-proof branch September 28, 2026 14:58
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.

2 participants