From aaa9ddc0d64ae10f97a691107a15ea52827bd987 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 10 Apr 2024 13:58:18 -0700 Subject: [PATCH] Check etching commit confirmations correctly (#3507) Previously, we were checking if an etching commit transaction had matured by asking bitcoind how many confirmations it had. This was incorrect, because if ord was behind bitcoind, the number of confirmations would be based on bitcoind's current block height, and would be greater than the number of confirmations the commit transaction had as of the reveal transaction block height. This commit fixes this by calculating the number of confirmations using the block height of the commit transaction, which is correct even if bitcoind is ahead. --- crates/mockcore/src/api.rs | 6 +++ crates/mockcore/src/server.rs | 44 ++++++++++++++-- crates/ordinals/src/runestone.rs | 2 +- src/index/testing.rs | 5 +- src/index/updater/rune_updater.rs | 27 +++++++--- src/runes.rs | 70 ++++++++++++++++++++++++-- src/subcommand/server.rs | 14 +++--- src/subcommand/wallet/batch_command.rs | 2 +- src/wallet.rs | 4 +- src/wallet/batch/plan.rs | 2 +- tests/wallet/batch_command.rs | 10 ++-- 11 files changed, 152 insertions(+), 34 deletions(-) diff --git a/crates/mockcore/src/api.rs b/crates/mockcore/src/api.rs index 2ebe549ea6..8571ec3ea3 100644 --- a/crates/mockcore/src/api.rs +++ b/crates/mockcore/src/api.rs @@ -25,6 +25,12 @@ pub trait Api { verbose: bool, ) -> Result; + #[rpc(name = "getblockheaderinfo")] + fn get_block_header_info( + &self, + block_hash: BlockHash, + ) -> Result; + #[rpc(name = "getblockstats")] fn get_block_stats(&self, height: usize) -> Result; diff --git a/crates/mockcore/src/server.rs b/crates/mockcore/src/server.rs index e9ed3fffbf..0fc13f6745 100644 --- a/crates/mockcore/src/server.rs +++ b/crates/mockcore/src/server.rs @@ -149,6 +149,36 @@ impl Api for Server { } } + fn get_block_header_info( + &self, + block_hash: BlockHash, + ) -> Result { + let state = self.state(); + + let height = match state.hashes.iter().position(|hash| *hash == block_hash) { + Some(height) => height, + None => return Err(Self::not_found()), + }; + + Ok(GetBlockHeaderResult { + height, + hash: block_hash, + confirmations: 0, + version: Version::ONE, + version_hex: None, + merkle_root: TxMerkleNode::all_zeros(), + time: 0, + median_time: None, + nonce: 0, + bits: String::new(), + difficulty: 0.0, + chainwork: Vec::new(), + n_tx: 0, + previous_block_hash: None, + next_block_hash: None, + }) + } + fn get_block_stats(&self, height: usize) -> Result { let Some(block_hash) = self.state().hashes.get(height).cloned() else { return Err(Self::not_found()); @@ -630,12 +660,16 @@ impl Api for Server { blockhash: Option, ) -> Result { assert_eq!(blockhash, None, "Blockhash param is unsupported"); + let state = self.state(); + let current_height: u32 = (state.hashes.len() - 1).try_into().unwrap(); - let confirmations = state - .txid_to_block_height - .get(&txid) - .map(|tx_height| current_height - tx_height); + + let tx_height = state.txid_to_block_height.get(&txid); + + let confirmations = tx_height.map(|tx_height| current_height - tx_height); + + let blockhash = tx_height.map(|tx_height| state.hashes[usize::try_from(*tx_height).unwrap()]); if verbose.unwrap_or(false) { match state.transactions.get(&txid) { @@ -667,7 +701,7 @@ impl Api for Server { }, }) .collect(), - blockhash: None, + blockhash, confirmations, time: None, blocktime: None, diff --git a/crates/ordinals/src/runestone.rs b/crates/ordinals/src/runestone.rs index 0de485b62a..a8b5c77fec 100644 --- a/crates/ordinals/src/runestone.rs +++ b/crates/ordinals/src/runestone.rs @@ -20,7 +20,7 @@ enum Payload { impl Runestone { pub const MAGIC_NUMBER: opcodes::All = opcodes::all::OP_PUSHNUM_13; - pub const COMMIT_INTERVAL: u16 = 6; + pub const COMMIT_CONFIRMATIONS: u16 = 6; pub fn decipher(transaction: &Transaction) -> Option { let payload = match Runestone::payload(transaction) { diff --git a/src/index/testing.rs b/src/index/testing.rs index 6513b9c5ab..ae4f02a823 100644 --- a/src/index/testing.rs +++ b/src/index/testing.rs @@ -161,7 +161,7 @@ impl Context { ..default() }); - self.mine_blocks(Runestone::COMMIT_INTERVAL.into()); + self.mine_blocks(Runestone::COMMIT_CONFIRMATIONS.into()); let mut witness = Witness::new(); @@ -197,7 +197,8 @@ impl Context { ( txid, RuneId { - block: u64::try_from(block_count + usize::from(Runestone::COMMIT_INTERVAL) + 1).unwrap(), + block: u64::try_from(block_count + usize::from(Runestone::COMMIT_CONFIRMATIONS) + 1) + .unwrap(), tx: 1, }, ) diff --git a/src/index/updater/rune_updater.rs b/src/index/updater/rune_updater.rs index 744ff587b9..ab5bcedcfb 100644 --- a/src/index/updater/rune_updater.rs +++ b/src/index/updater/rune_updater.rs @@ -388,7 +388,10 @@ impl<'a, 'tx, 'client> RuneUpdater<'a, 'tx, 'client> { .get_raw_transaction_info(&input.previous_output.txid, None) .into_option()? else { - panic!("input not in UTXO set: {}", input.previous_output); + panic!( + "can't get input transaction: {}", + input.previous_output.txid + ); }; let taproot = tx_info.vout[input.previous_output.vout.into_usize()] @@ -396,12 +399,24 @@ impl<'a, 'tx, 'client> RuneUpdater<'a, 'tx, 'client> { .script()? .is_v1_p2tr(); - let mature = tx_info - .confirmations - .map(|confirmations| confirmations >= Runestone::COMMIT_INTERVAL.into()) - .unwrap_or_default(); + if !taproot { + continue; + } + + let commit_tx_height = self + .client + .get_block_header_info(&tx_info.blockhash.unwrap()) + .into_option()? + .unwrap() + .height; + + let confirmations = self + .height + .checked_sub(commit_tx_height.try_into().unwrap()) + .unwrap() + + 1; - if taproot && mature { + if confirmations >= Runestone::COMMIT_CONFIRMATIONS.into() { return Ok(true); } } diff --git a/src/runes.rs b/src/runes.rs index 3919273fb3..1e22229c06 100644 --- a/src/runes.rs +++ b/src/runes.rs @@ -147,7 +147,7 @@ mod tests { fn runes_must_be_greater_than_or_equal_to_minimum_for_height() { let minimum = Rune::minimum_at_height( Chain::Regtest.network(), - Height((Runestone::COMMIT_INTERVAL + 2).into()), + Height((Runestone::COMMIT_CONFIRMATIONS + 2).into()), ) .0; @@ -5196,7 +5196,7 @@ mod tests { ..default() }); - context.mine_blocks(Runestone::COMMIT_INTERVAL.into()); + context.mine_blocks(Runestone::COMMIT_CONFIRMATIONS.into()); let mut witness = Witness::new(); @@ -5256,7 +5256,7 @@ mod tests { ..default() }); - context.mine_blocks((Runestone::COMMIT_INTERVAL - 1).into()); + context.mine_blocks((Runestone::COMMIT_CONFIRMATIONS - 2).into()); let mut witness = Witness::new(); @@ -5302,6 +5302,68 @@ mod tests { context.assert_runes([], []); } + #[test] + fn immature_commits_are_not_valid_even_when_bitcoind_is_ahead() { + let context = Context::builder().arg("--index-runes").build(); + + let block_count = context.index.block_count().unwrap().into_usize(); + + context.mine_blocks_with_update(1, false); + + context.core.broadcast_tx(TransactionTemplate { + inputs: &[(block_count, 0, 0, Witness::new())], + p2tr: true, + ..default() + }); + + context.mine_blocks_with_update((Runestone::COMMIT_CONFIRMATIONS - 2).into(), false); + + let mut witness = Witness::new(); + + let runestone = Runestone { + etching: Some(Etching { + rune: Some(Rune(RUNE)), + terms: Some(Terms { + amount: Some(1000), + ..default() + }), + ..default() + }), + ..default() + }; + + let tapscript = script::Builder::new() + .push_slice::<&PushBytes>( + runestone + .etching + .unwrap() + .rune + .unwrap() + .commitment() + .as_slice() + .try_into() + .unwrap(), + ) + .into_script(); + + witness.push(tapscript); + + witness.push([]); + + context.core.broadcast_tx(TransactionTemplate { + inputs: &[(block_count + 1, 1, 0, witness)], + op_return: Some(runestone.encipher()), + outputs: 1, + ..default() + }); + + context.mine_blocks_with_update(2, false); + + context.mine_blocks_with_update(1, true); + + context.assert_runes([], []); + } + #[test] fn etchings_are_not_valid_without_commitment() { let context = Context::builder().arg("--index-runes").build(); @@ -5316,7 +5378,7 @@ mod tests { ..default() }); - context.mine_blocks(Runestone::COMMIT_INTERVAL.into()); + context.mine_blocks(Runestone::COMMIT_CONFIRMATIONS.into()); let mut witness = Witness::new(); diff --git a/src/subcommand/server.rs b/src/subcommand/server.rs index f24563dafc..e89fd4782a 100644 --- a/src/subcommand/server.rs +++ b/src/subcommand/server.rs @@ -2078,7 +2078,7 @@ mod tests { ..default() }); - self.mine_blocks(Runestone::COMMIT_INTERVAL.into()); + self.mine_blocks((Runestone::COMMIT_CONFIRMATIONS - 1).into()); let witness = witness.unwrap_or_else(|| { let tapscript = script::Builder::new() @@ -2535,8 +2535,8 @@ mod tests { server.mine_blocks(1); - server.assert_redirect("/search/9:1", "/rune/AAAAAAAAAAAAA"); - server.assert_redirect("/search?query=9:1", "/rune/AAAAAAAAAAAAA"); + server.assert_redirect("/search/8:1", "/rune/AAAAAAAAAAAAA"); + server.assert_redirect("/search?query=8:1", "/rune/AAAAAAAAAAAAA"); server.assert_response_regex( "/search/100000000000000000000:200000000000000000", @@ -2616,7 +2616,7 @@ mod tests { server.mine_blocks(1); server.assert_response_regex( - "/rune/9:1", + "/rune/8:1", StatusCode::OK, ".*Rune AAAAAAAAAAAAA.*", ); @@ -2763,11 +2763,11 @@ mod tests {
number
0
timestamp
-
+
id
-
9:1
+
8:1
etching block
-
9
+
8
etching transaction
1
mint
diff --git a/src/subcommand/wallet/batch_command.rs b/src/subcommand/wallet/batch_command.rs index 486789b298..a5f9123888 100644 --- a/src/subcommand/wallet/batch_command.rs +++ b/src/subcommand/wallet/batch_command.rs @@ -120,7 +120,7 @@ impl Batch { let current_height = u32::try_from(bitcoin_client.get_block_count()?).unwrap(); - let reveal_height = current_height + 1 + u32::from(Runestone::COMMIT_INTERVAL); + let reveal_height = current_height + u32::from(Runestone::COMMIT_CONFIRMATIONS); if let Some(terms) = etching.terms { if let Some((start, end)) = terms.offset.and_then(|range| range.start.zip(range.end)) { diff --git a/src/wallet.rs b/src/wallet.rs index 0151376f5c..02dd6a8907 100644 --- a/src/wallet.rs +++ b/src/wallet.rs @@ -320,14 +320,14 @@ impl Wallet { if let Some(transaction) = transaction { if u32::try_from(transaction.info.confirmations).unwrap() - >= Runestone::COMMIT_INTERVAL.into() + >= Runestone::COMMIT_CONFIRMATIONS.into() { let tx_out = self .bitcoin_client() .get_tx_out(&commit.txid(), 0, Some(true))?; if let Some(tx_out) = tx_out { - if tx_out.confirmations >= Runestone::COMMIT_INTERVAL.into() { + if tx_out.confirmations >= Runestone::COMMIT_CONFIRMATIONS.into() { break; } } else { diff --git a/src/wallet/batch/plan.rs b/src/wallet/batch/plan.rs index 1f51127464..a8e7040a65 100644 --- a/src/wallet/batch/plan.rs +++ b/src/wallet/batch/plan.rs @@ -711,7 +711,7 @@ impl Plan { script_sig: script::Builder::new().into_script(), witness: Witness::new(), sequence: if etching { - Sequence::from_height(Runestone::COMMIT_INTERVAL) + Sequence::from_height(Runestone::COMMIT_CONFIRMATIONS - 1) } else { Sequence::ENABLE_RBF_NO_LOCKTIME }, diff --git a/tests/wallet/batch_command.rs b/tests/wallet/batch_command.rs index 2b8293f910..bc530b8284 100644 --- a/tests/wallet/batch_command.rs +++ b/tests/wallet/batch_command.rs @@ -1480,7 +1480,7 @@ fn batch_can_etch_rune() { assert_eq!( reveal.input[0].sequence, - Sequence::from_height(Runestone::COMMIT_INTERVAL) + Sequence::from_height(Runestone::COMMIT_CONFIRMATIONS - 1) ); let Artifact::Runestone(runestone) = Runestone::decipher(&reveal).unwrap() else { @@ -1577,7 +1577,7 @@ fn batch_can_etch_rune_without_premine() { assert_eq!( reveal.input[0].sequence, - Sequence::from_height(Runestone::COMMIT_INTERVAL) + Sequence::from_height(Runestone::COMMIT_CONFIRMATIONS - 1) ); assert_eq!( @@ -1859,7 +1859,7 @@ fn etch_sub_minimum_rune_error() { ) .core(&core) .ord(&ord) - .expected_stderr("error: rune is less than minimum for next block: A < ZZQYZPATYGGX\n") + .expected_stderr("error: rune is less than minimum for next block: A < ZZRZCNJJBILX\n") .expected_exit_code(1) .run_and_extract_stdout(); } @@ -2236,7 +2236,7 @@ fn invalid_start_height_error() { .core(&core) .ord(&ord) .expected_stderr( - "error: `terms.height.start` must be greater than the reveal transaction block height of 8\n", + "error: `terms.height.start` must be greater than the reveal transaction block height of 7\n", ) .expected_exit_code(1) .run_and_extract_stdout(); @@ -2287,7 +2287,7 @@ fn invalid_end_height_error() { .core(&core) .ord(&ord) .expected_stderr( - "error: `terms.height.end` must be greater than the reveal transaction block height of 8\n", + "error: `terms.height.end` must be greater than the reveal transaction block height of 7\n", ) .expected_exit_code(1) .run_and_extract_stdout();