Skip to content

smite: add transaction block position lookup to BitcoinCli#140

Open
devvaansh wants to merge 2 commits into
morehouse:masterfrom
devvaansh:bitcoin-block-position-lookup
Open

smite: add transaction block position lookup to BitcoinCli#140
devvaansh wants to merge 2 commits into
morehouse:masterfrom
devvaansh:bitcoin-block-position-lookup

Conversation

@devvaansh

Copy link
Copy Markdown
Contributor

Adds BitcoinCli::get_transaction_block_position, which returns the (block_height, tx_index) pair for a confirmed transaction by issuing getrawtransaction followed by getblock. Returns None for unconfirmed or unknown transactions.

This is the missing piece needed to derive a BOLT 7 short_channel_id from a mined 2-of-2 P2WSH funding output, and is a prerequisite for the LookupShortChannelId IR operation that will let ChannelAnnouncementGenerator pass on-chain UTXO validation in CLN and LND.

Ref: #71

Add BitcoinCli::get_transaction_block_position, which returns the
(block_height, tx_index) pair for a confirmed transaction. This is the
information a fuzzer needs to derive the BOLT 7 short_channel_id of a
mined 2-of-2 P2WSH funding output, and is a prerequisite for driving
gossip messages past on-chain UTXO validation in CLN and LND.

Signed-off-by: Devansh Vashisht <devansh.vashisht.ug24@nsut.ac.in>

@NishantBansal2003 NishantBansal2003 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.

ACK

I assume that, with this, we’re only left with operation like ComputeShortChannelId, which resolve a funding tx, retrieve its block height and tx index, and compute the SCID from them? In that case, we also need to handle the case where the funding tx hasn't been confirmed yet

Comment thread smite/src/bitcoin.rs Outdated
@devvaansh

Copy link
Copy Markdown
Contributor Author

ACK

I assume that, with this, we’re only left with operation like ComputeShortChannelId, which resolve a funding tx, retrieve its block height and tx index, and compute the SCID from them? In that case, we also need to handle the case where the funding tx hasn't been confirmed yet

Yes, next PR adds LookupShortChannelId (input: FundingTransaction → output: ShortChannelId). The generator always emits BroadcastTransactionMineBlocks(6)LookupShortChannelId in sequence, so the tx is confirmed by the time the op executes. If a mutator deletes MineBlocks, get_transaction_block_position returns None and the executor panics, same behaviour as any other broken program (type mismatch, bad variable ref, etc.). No special handling needed.

@NishantBansal2003

Copy link
Copy Markdown
Contributor

Yes, next PR adds LookupShortChannelId (input: FundingTransaction → output: ShortChannelId). The generator always emits BroadcastTransactionMineBlocks(6)LookupShortChannelId in sequence, so the tx is confirmed by the time the op executes. If a mutator deletes MineBlocks, get_transaction_block_position returns None and the executor panics, same behaviour as any other broken program (type mismatch, bad variable ref, etc.). No special handling needed.

I don’t think we should make the generator mine a specific number of blocks. I think we should keep it as a range, like rng.random_range(1..=16), in the generator as well

If the tx is not confirmed, that's still a valid program, so it shouldn't panic. smite should only panic when the program (generated by smite) is invalid, in which case it’s a bug that should be fixed in smite-ir itself

@morehouse morehouse left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please squash to a single commit.

Comment thread smite/src/bitcoin.rs
Comment on lines +293 to +298
#[derive(Deserialize)]
struct GetRawTransactionResponse {
// Omitted by `getrawtransaction` while the transaction is unconfirmed
// (in the mempool).
blockhash: Option<String>,
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We now have two functions that call getrawtransaction. It would seem natural to have a private helper that returns the GetRawTransactionResponse.

Comment thread smite/src/bitcoin.rs
Comment on lines +329 to +334
let block_out = self
.run()
.arg("getblock")
.arg(&blockhash)
.output()
.expect("bitcoin-cli getblock should not fail");

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It might be good to explicitly set verbosity=1 so that this doesn't break if the default verbosity ever changes.

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.

3 participants