Skip to content

Commit

Permalink
Check etching commit confirmations correctly (ordinals#3507)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
casey committed Apr 11, 2024
1 parent 3c4a257 commit aaa9ddc
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 34 deletions.
6 changes: 6 additions & 0 deletions crates/mockcore/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ pub trait Api {
verbose: bool,
) -> Result<Value, jsonrpc_core::Error>;

#[rpc(name = "getblockheaderinfo")]
fn get_block_header_info(
&self,
block_hash: BlockHash,
) -> Result<GetBlockHeaderResult, jsonrpc_core::Error>;

#[rpc(name = "getblockstats")]
fn get_block_stats(&self, height: usize) -> Result<GetBlockStatsResult, jsonrpc_core::Error>;

Expand Down
44 changes: 39 additions & 5 deletions crates/mockcore/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,36 @@ impl Api for Server {
}
}

fn get_block_header_info(
&self,
block_hash: BlockHash,
) -> Result<GetBlockHeaderResult, jsonrpc_core::Error> {
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<GetBlockStatsResult, jsonrpc_core::Error> {
let Some(block_hash) = self.state().hashes.get(height).cloned() else {
return Err(Self::not_found());
Expand Down Expand Up @@ -630,12 +660,16 @@ impl Api for Server {
blockhash: Option<BlockHash>,
) -> Result<Value, jsonrpc_core::Error> {
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) {
Expand Down Expand Up @@ -667,7 +701,7 @@ impl Api for Server {
},
})
.collect(),
blockhash: None,
blockhash,
confirmations,
time: None,
blocktime: None,
Expand Down
2 changes: 1 addition & 1 deletion crates/ordinals/src/runestone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Artifact> {
let payload = match Runestone::payload(transaction) {
Expand Down
5 changes: 3 additions & 2 deletions src/index/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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,
},
)
Expand Down
27 changes: 21 additions & 6 deletions src/index/updater/rune_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,20 +388,35 @@ 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()]
.script_pub_key
.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);
}
}
Expand Down
70 changes: 66 additions & 4 deletions src/runes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand All @@ -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();

Expand Down
14 changes: 7 additions & 7 deletions src/subcommand/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -2616,7 +2616,7 @@ mod tests {
server.mine_blocks(1);

server.assert_response_regex(
"/rune/9:1",
"/rune/8:1",
StatusCode::OK,
".*<title>Rune AAAAAAAAAAAAA</title>.*",
);
Expand Down Expand Up @@ -2763,11 +2763,11 @@ mod tests {
<dt>number</dt>
<dd>0</dd>
<dt>timestamp</dt>
<dd><time>1970-01-01 00:00:09 UTC</time></dd>
<dd><time>1970-01-01 00:00:08 UTC</time></dd>
<dt>id</dt>
<dd>9:1</dd>
<dd>8:1</dd>
<dt>etching block</dt>
<dd><a href=/block/9>9</a></dd>
<dd><a href=/block/8>8</a></dd>
<dt>etching transaction</dt>
<dd>1</dd>
<dt>mint</dt>
Expand Down
2 changes: 1 addition & 1 deletion src/subcommand/wallet/batch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
4 changes: 2 additions & 2 deletions src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/batch/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
10 changes: 5 additions & 5 deletions tests/wallet/batch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit aaa9ddc

Please sign in to comment.