smite-ir: add LookupShortChannelId operation - #155
Conversation
5082f5e to
779e50b
Compare
morehouse
left a comment
There was a problem hiding this comment.
Just missing an executor test.
| pub fn has_side_effects(&self) -> bool { | ||
| match self { | ||
| Self::SendMessage | ||
| | Self::SendOpenChannel | ||
| | Self::SendFundingCreated | ||
| | Self::SendChannelReady { .. } | ||
| | Self::RecvAcceptChannel | ||
| | Self::RecvFundingSigned | ||
| | Self::RecvChannelReady | ||
| | Self::MineBlocks(_) | ||
| | Self::CreateFundingTransaction | ||
| | Self::BroadcastTransaction => true, | ||
| | Self::BroadcastTransaction | ||
| | Self::LookupShortChannelId => true, |
There was a problem hiding this comment.
I would argue that LookupShortChannelId does not have side effects, since it is a read-only operation. But I see that we need it here to prevent CSE from merging it with other lookups, since the result is nondeterministic (depends on the relative position of a MineBlocks operation).
I think it's fine to put it here to keep this PR simple. But this inconsistency hints that we may want a separate is_deterministic function for use by CSE, which could be a follow-up PR.
There was a problem hiding this comment.
Agreed. It's purely a read- no I/O, no state mutation. The only reason it's in has_side_effects is to keep CSE from collapsing two lookups that straddle a MineBlocks. Will track is_deterministic as a follow-up.
779e50b to
86782fe
Compare
86782fe to
c397439
Compare
a2bb508 to
4fac511
Compare
Adds an IR operation that takes a broadcast FundingTransaction and produces the BOLT 7 short_channel_id derived from its confirmed block position. This bridges the on-chain funding output to the gossip layer so channel_announcement, channel_update, and announcement_signatures messages can reference a real UTXO and pass on-chain validation in CLN and LND. Under the hood the executor calls BitcoinCli::get_transaction_block_position and combines the returned (block_height, tx_index) with the funding transaction's vout. If the transaction is unknown to the node or still in the mempool (e.g. a mutator dropped MineBlocks between the broadcast and the lookup), the sentinel ShortChannelId::new(0, 0, 0) is produced instead of panicking: the resulting gossip message simply fails on-chain validation, which is the desired behaviour for a valid but unconfirmed program. The BitcoinRpc trait grows a get_transaction_block_position method with a MockBitcoinCli implementation that mirrors the existing confirmations mock.
4fac511 to
f76879f
Compare
Adds
LookupShortChannelId, which takes aFundingTransactionand produces the BOLT 7short_channel_idderived from its confirmed block position.This is the missing piece needed to build validly-signed gossip messages that reference a real on-chain UTXO and can pass the UTXO validation checks in CLN and LND.
If the transaction is not yet confirmed (e.g. a mutator dropped
MineBlocks), the sentinelShortChannelId::new(0, 0, 0)is returned rather than panicking. A valid but unconfirmed program should not be a fatal error.Ref: #71