From 50027920222f8acbdf37c155cfbc5ffc0f4efd79 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Tue, 18 Apr 2023 16:54:05 -0400 Subject: [PATCH] feat: add block height to `commit-block` event The event generated by a `commit-block` (on the L1) now includes the block height for the block that is being committed, in the `block-height` key of the event tuple. The target burnchain block height now uses the key `target-burn-block-height`. This is useful for debugging, making it easy to match the L1 transactions with the L2 blocks. Note that this field is not checked in the tests since it is purely for convenience and could be left out of some implementations. --- core-contracts/contracts/multi-miner.clar | 6 ++-- core-contracts/contracts/subnet.clar | 29 +++++++++++++------ .../tests/subnets/multiminer_test.ts | 5 ++++ core-contracts/tests/subnets/subnet_test.ts | 24 +++++++++++---- src/chainstate/stacks/miner.rs | 2 ++ .../stacks-node/src/burnchains/commitment.rs | 16 ++++++++++ .../stacks-node/src/burnchains/l1_events.rs | 2 ++ .../stacks-node/src/burnchains/mock_events.rs | 1 + testnet/stacks-node/src/burnchains/mod.rs | 2 ++ testnet/stacks-node/src/neon_node.rs | 4 +-- 10 files changed, 72 insertions(+), 19 deletions(-) diff --git a/core-contracts/contracts/multi-miner.clar b/core-contracts/contracts/multi-miner.clar index e07a1182c..932c292ad 100644 --- a/core-contracts/contracts/multi-miner.clar +++ b/core-contracts/contracts/multi-miner.clar @@ -55,7 +55,7 @@ (asserts! (>= (len provided-checked) signers-required) (err ERR_NOT_ENOUGH_SIGNERS)) (ok true))) -(define-read-only (make-block-commit-hash (block-data { block: (buff 32), withdrawal-root: (buff 32), target-tip: (buff 32) })) +(define-read-only (make-block-commit-hash (block-data { block: (buff 32), subnet-block-height: uint, withdrawal-root: (buff 32), target-tip: (buff 32) })) (let ((data-buff (unwrap-panic (to-consensus-buff? (merge block-data { multi-contract: CONTRACT_ADDRESS })))) (data-hash (sha256 data-buff)) ;; in 2.0, this is a constant: 0xe2f4d0b1eca5f1b4eb853cd7f1c843540cfb21de8bfdaa59c504a6775cd2cfe9 @@ -74,7 +74,7 @@ signers: (unwrap-panic (as-max-len? (append (get signers prior-okay) curr-signer) u9)) })) prior-err (err prior-err))) -(define-public (commit-block (block-data { block: (buff 32), withdrawal-root: (buff 32), target-tip: (buff 32) }) +(define-public (commit-block (block-data { block: (buff 32), subnet-block-height: uint, withdrawal-root: (buff 32), target-tip: (buff 32) }) (signatures (list 9 (buff 65)))) (let ((block-data-hash (make-block-commit-hash block-data)) (signer-principals (try! (fold verify-sign-helper signatures (ok { block-hash: block-data-hash, signers: (list) }))))) @@ -83,6 +83,6 @@ ;; check that we have enough signatures (try! (check-miners (append (get signers signer-principals) tx-sender))) ;; execute the block commit - (as-contract (contract-call? .subnet commit-block (get block block-data) (get target-tip block-data) (get withdrawal-root block-data))) + (as-contract (contract-call? .subnet commit-block (get block block-data) (get subnet-block-height block-data) (get target-tip block-data) (get withdrawal-root block-data))) ) ) diff --git a/core-contracts/contracts/subnet.clar b/core-contracts/contracts/subnet.clar index 971fcdd77..56be369c8 100644 --- a/core-contracts/contracts/subnet.clar +++ b/core-contracts/contracts/subnet.clar @@ -109,10 +109,10 @@ ;; Helper function: determines whether the commit-block operation satisfies pre-conditions ;; listed in `commit-block`. ;; Returns response -(define-private (can-commit-block? (commit-block-height uint) (target-chain-tip (buff 32))) +(define-private (can-commit-block? (l1-block-height uint) (target-chain-tip (buff 32))) (begin ;; check no block has been committed at this height - (asserts! (is-none (map-get? block-commits commit-block-height)) (err ERR_BLOCK_ALREADY_COMMITTED)) + (asserts! (is-none (map-get? block-commits l1-block-height)) (err ERR_BLOCK_ALREADY_COMMITTED)) ;; check that `target-chain-tip` matches the burn chain tip (asserts! (is-eq @@ -132,15 +132,21 @@ ;; Helper function: modifies the block-commits map with a new commit and prints related info ;; Returns response<(buff 32), ?> -(define-private (inner-commit-block (block (buff 32)) (commit-block-height uint) (withdrawal-root (buff 32))) +(define-private (inner-commit-block + (block (buff 32)) + (subnet-block-height uint) + (l1-block-height uint) + (withdrawal-root (buff 32)) + ) (begin - (map-set block-commits commit-block-height block) + (map-set block-commits l1-block-height block) (map-set withdrawal-roots-map withdrawal-root true) (print { event: "block-commit", block-commit: block, + subnet-block-height: subnet-block-height, withdrawal-root: withdrawal-root, - block-height: commit-block-height + l1-block-height: l1-block-height }) (ok block) ) @@ -155,10 +161,15 @@ ;; 1) we have already committed at this block height ;; 2) `target-chain-tip` is not the burn chain tip (i.e., on this chain) ;; 3) the sender is not a miner -(define-public (commit-block (block (buff 32)) (target-chain-tip (buff 32)) (withdrawal-root (buff 32))) - (let ((commit-block-height block-height)) - (try! (can-commit-block? commit-block-height target-chain-tip)) - (inner-commit-block block commit-block-height withdrawal-root) +(define-public (commit-block + (block (buff 32)) + (subnet-block-height uint) + (target-chain-tip (buff 32)) + (withdrawal-root (buff 32)) + ) + (let ((l1-block-height block-height)) + (try! (can-commit-block? l1-block-height target-chain-tip)) + (inner-commit-block block subnet-block-height l1-block-height withdrawal-root) ) ) diff --git a/core-contracts/tests/subnets/multiminer_test.ts b/core-contracts/tests/subnets/multiminer_test.ts index f3b61ba5d..c11898780 100644 --- a/core-contracts/tests/subnets/multiminer_test.ts +++ b/core-contracts/tests/subnets/multiminer_test.ts @@ -112,6 +112,7 @@ Clarinet.test({ .toString(); const commit_data_1 = types.tuple({ block: buffFromHex(block_hash_1.slice(2)), + "subnet-block-height": types.uint(0), "withdrawal-root": buffFromHex(withdrawal_root_1.slice(2)), "target-tip": id_header_hash_1, }); @@ -149,6 +150,7 @@ Clarinet.test({ .toString(); const commit_data_2 = types.tuple({ block: buffFromHex(block_hash_1.slice(2)), + "subnet-block-height": types.uint(1), "withdrawal-root": buffFromHex(withdrawal_root_1.slice(2)), "target-tip": id_header_hash_2, }); @@ -278,6 +280,7 @@ Clarinet.test({ .toString(); const commit_data_1 = types.tuple({ block: buffFromHex(block_hash_1.slice(2)), + "subnet-block-height": types.uint(0), "withdrawal-root": buffFromHex(withdrawal_root_1.slice(2)), "target-tip": id_header_hash_1, }); @@ -317,6 +320,7 @@ Clarinet.test({ .toString(); const commit_data_2 = types.tuple({ block: buffFromHex(block_hash_1.slice(2)), + "subnet-block-height": types.uint(0), "withdrawal-root": buffFromHex(withdrawal_root_1.slice(2)), "target-tip": id_header_hash_2, }); @@ -357,6 +361,7 @@ Clarinet.test({ .toString(); const commit_data_3 = types.tuple({ block: buffFromHex(block_hash_1.slice(2)), + "subnet-block-height": types.uint(0), "withdrawal-root": buffFromHex(withdrawal_root_1.slice(2)), "target-tip": id_header_hash_3, }); diff --git a/core-contracts/tests/subnets/subnet_test.ts b/core-contracts/tests/subnets/subnet_test.ts index c24bf091f..277a9d42f 100644 --- a/core-contracts/tests/subnets/subnet_test.ts +++ b/core-contracts/tests/subnets/subnet_test.ts @@ -8,9 +8,7 @@ import { } from "https://deno.land/x/clarinet@v1.2.0/index.ts"; import { assertEquals } from "https://deno.land/std@0.90.0/testing/asserts.ts"; -import { - decode as decHex, -} from "https://deno.land/std@0.149.0/encoding/hex.ts"; +import { decode as decHex } from "https://deno.land/std@0.149.0/encoding/hex.ts"; function fromHex(input: string) { const hexBytes = new TextEncoder().encode(input); @@ -135,6 +133,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash1, types.buff(new Uint8Array([0, 1, 1, 1, 2])), ], @@ -146,6 +145,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 2, 2, 2, 2])), + types.uint(1), id_header_hash1, types.buff(new Uint8Array([0, 2, 2, 2, 3])), ], @@ -170,6 +170,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 2, 2, 2, 2])), + types.uint(1), id_header_hash2, types.buff(new Uint8Array([0, 2, 2, 2, 3])), ], @@ -181,6 +182,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 2, 2, 2, 1])), + types.uint(1), types.buff(new Uint8Array([0, 2, 2, 2, 2])), types.buff(new Uint8Array([0, 2, 2, 2, 3])), ], @@ -203,6 +205,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 2, 2, 2, 2])), + types.uint(1), id_header_hash3, types.buff(new Uint8Array([0, 2, 2, 2, 3])), ], @@ -429,7 +432,6 @@ Clarinet.test({ charlie.address ), ]); - console.log("BLOCK:", block); // should return (err ERR_DISALLOWED_ASSET) block.receipts[0].result.expectErr().expectInt(5); @@ -523,6 +525,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -712,6 +715,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -1100,6 +1104,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -1295,6 +1300,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -1622,6 +1628,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -1919,6 +1926,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -2210,6 +2218,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -2319,6 +2328,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -2426,7 +2436,8 @@ Clarinet.test({ block.receipts[0].result.expectOk().expectBool(true); // Check that user does not own this NFT on the L1 - const assets = chain.getAssetsMaps().assets[".simple-nft-no-mint.nft-token"]; + const assets = + chain.getAssetsMaps().assets[".simple-nft-no-mint.nft-token"]; assertEquals(assets, undefined); // Miner should commit a block with the appropriate root hash (mocking a withdrawal Merkle tree) @@ -2451,6 +2462,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -2585,6 +2597,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], @@ -2736,6 +2749,7 @@ Clarinet.test({ "commit-block", [ types.buff(new Uint8Array([0, 1, 1, 1, 1])), + types.uint(0), id_header_hash, types.buff(root_hash), ], diff --git a/src/chainstate/stacks/miner.rs b/src/chainstate/stacks/miner.rs index 77c17ec5e..27187b699 100644 --- a/src/chainstate/stacks/miner.rs +++ b/src/chainstate/stacks/miner.rs @@ -2342,6 +2342,7 @@ impl Proposal { // when using a 2.1 layer-1, this will need to use the structured data hash let block_hash_buff = Value::buff_from(self.block.block_hash().0.to_vec()) .expect("Failed to form Clarity buffer from block hash"); + let subnet_block_height = Value::UInt(self.block.header.total_work.work.into()); let withdrawal_root_buff = Value::buff_from(self.block.header.withdrawal_merkle_root.0.to_vec()) .expect("Failed to form Clarity buffer from withdrawal root"); @@ -2352,6 +2353,7 @@ impl Proposal { let data_tuple = Value::Tuple( TupleData::from_data(vec![ ("block".into(), block_hash_buff), + ("subnet-block-height".into(), subnet_block_height), ("withdrawal-root".into(), withdrawal_root_buff), ("target-tip".into(), target_tip), ("multi-contract".into(), signing_contract), diff --git a/testnet/stacks-node/src/burnchains/commitment.rs b/testnet/stacks-node/src/burnchains/commitment.rs index 9a01fb876..9a41db90b 100644 --- a/testnet/stacks-node/src/burnchains/commitment.rs +++ b/testnet/stacks-node/src/burnchains/commitment.rs @@ -40,6 +40,7 @@ pub trait Layer1Committer { fn make_commit_tx( &self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, signatures: Vec, @@ -231,6 +232,7 @@ impl MultiPartyCommitter { sender_nonce: u64, tx_fee: u64, commit_to: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_root: Sha512Trunc256Sum, signatures: Vec, @@ -247,6 +249,7 @@ impl MultiPartyCommitter { let block_val = ClarityValue::buff_from(commit_to.as_bytes().to_vec()) .map_err(|_| Error::BadCommitment)?; + let height_val = ClarityValue::UInt(committed_block_height.into()); let target_tip_val = ClarityValue::buff_from(target_tip.as_bytes().to_vec()) .map_err(|_| Error::BadCommitment)?; let withdrawal_root_val = ClarityValue::buff_from(withdrawal_root.as_bytes().to_vec()) @@ -264,6 +267,7 @@ impl MultiPartyCommitter { let block_data_val = TupleData::from_data(vec![ ("block".into(), block_val), + ("subnet-block-height".into(), height_val), ("withdrawal-root".into(), withdrawal_root_val), ("target-tip".into(), target_tip_val), ]) @@ -300,6 +304,7 @@ impl MultiPartyCommitter { pub fn make_commit_tx( &self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, signatures: Vec, @@ -325,6 +330,7 @@ impl MultiPartyCommitter { nonce, DEFAULT_MINER_COMMITMENT_FEE, committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, signatures.clone(), @@ -347,6 +353,7 @@ impl MultiPartyCommitter { nonce, computed_fee, committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, signatures, @@ -427,6 +434,7 @@ impl Layer1Committer for MultiPartyCommitter { fn make_commit_tx( &self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, signatures: Vec, @@ -435,6 +443,7 @@ impl Layer1Committer for MultiPartyCommitter { ) -> Result { self.make_commit_tx( committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, signatures, @@ -452,6 +461,7 @@ impl Layer1Committer for DirectCommitter { fn make_commit_tx( &self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, _signatures: Vec, @@ -460,6 +470,7 @@ impl Layer1Committer for DirectCommitter { ) -> Result { self.make_commit_tx( committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, attempt, @@ -483,6 +494,7 @@ impl DirectCommitter { sender_nonce: u64, tx_fee: u64, commit_to: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_root: Sha512Trunc256Sum, ) -> Result { @@ -504,6 +516,7 @@ impl DirectCommitter { function_name: ClarityName::from("commit-block"), function_args: vec![ ClarityValue::buff_from(committed_block).map_err(|_| Error::BadCommitment)?, + ClarityValue::UInt(committed_block_height.into()), ClarityValue::buff_from(target_tip_bytes).map_err(|_| Error::BadCommitment)?, ClarityValue::buff_from(withdrawal_root_bytes).map_err(|_| Error::BadCommitment)?, ], @@ -533,6 +546,7 @@ impl DirectCommitter { pub fn make_commit_tx( &self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, attempt: u64, @@ -557,6 +571,7 @@ impl DirectCommitter { nonce, DEFAULT_MINER_COMMITMENT_FEE, committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, ) @@ -578,6 +593,7 @@ impl DirectCommitter { nonce, computed_fee, committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, ) diff --git a/testnet/stacks-node/src/burnchains/l1_events.rs b/testnet/stacks-node/src/burnchains/l1_events.rs index 627180776..1f911e01c 100644 --- a/testnet/stacks-node/src/burnchains/l1_events.rs +++ b/testnet/stacks-node/src/burnchains/l1_events.rs @@ -263,6 +263,7 @@ impl BurnchainController for L1Controller { fn submit_commit( &mut self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_tip: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, signatures: Vec, @@ -271,6 +272,7 @@ impl BurnchainController for L1Controller { ) -> Result { let tx = self.committer.make_commit_tx( committed_block_hash, + committed_block_height, target_tip, withdrawal_merkle_root, signatures, diff --git a/testnet/stacks-node/src/burnchains/mock_events.rs b/testnet/stacks-node/src/burnchains/mock_events.rs index 2772ace8e..d51b72179 100644 --- a/testnet/stacks-node/src/burnchains/mock_events.rs +++ b/testnet/stacks-node/src/burnchains/mock_events.rs @@ -418,6 +418,7 @@ impl BurnchainController for MockController { fn submit_commit( &mut self, committed_block_hash: BlockHeaderHash, + _committed_block_height: u64, _target_block: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, _signatures: Vec, diff --git a/testnet/stacks-node/src/burnchains/mod.rs b/testnet/stacks-node/src/burnchains/mod.rs index e975fa29f..44ba747b4 100644 --- a/testnet/stacks-node/src/burnchains/mod.rs +++ b/testnet/stacks-node/src/burnchains/mod.rs @@ -94,6 +94,7 @@ pub trait BurnchainController { fn submit_commit( &mut self, committed_block_hash: BlockHeaderHash, + committed_block_height: u64, target_burn_chain: BurnchainHeaderHash, withdrawal_merkle_root: Sha512Trunc256Sum, signatures: Vec, @@ -202,6 +203,7 @@ impl BurnchainController for PanicController { fn submit_commit( &mut self, _committed_block_hash: BlockHeaderHash, + _committed_block_height: u64, _target_block: BurnchainHeaderHash, _withdrawal_merkle_root: Sha512Trunc256Sum, _signatures: Vec, diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index 77ec2dd99..317f5fb5c 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -2003,6 +2003,7 @@ impl StacksNode { let res = bitcoin_controller.submit_commit( committed_block_hash, + block_height, target_burn_hash, withdrawal_merkle_root, signatures, @@ -2017,10 +2018,9 @@ impl StacksNode { Err(e) => { if !config.node.mock_mining { warn!("Failed to submit miner commitment L1 transaction: {}", e); - warn!("Failed to submit Bitcoin transaction"); return None; } else { - debug!("Mock-mining enabled; not sending Bitcoin transaction"); + debug!("Mock-mining enabled; not sending L1 transaction"); } } }