smite: mine consensus-valid transactions that violate mempool policy by NishantBansal2003 · Pull Request #137 · morehouse/smite

NishantBansal2003 · 2026-06-29T19:35:49Z

There were a couple of crashes while I was running the funding-flow generator, where we were getting panics in smite because of the restrictive assert statements we have set. These commits fix those issues in smite.

So now we can’t create two transactions with overlapping inputs. We return early when signing or broadcasting a transaction if it has already been broadcast and confirmed, since the wallet can’t sign a transaction whose inputs have already been spent. There were also a couple of crashes around broadcasting, where either funding_satoshis itself was below the dust limit, or the feerate used to calculate the transaction fee was too low compared to bitcoind default policy. So now we reject those inputs as well.

These cases do not really contribute to fuzzing, because in practice, if bitcoind default policy rejects them, Lightning nodes should also reject those open_channel messages before sending accept_channel. But let’s say they are not rejected, then I think those cases can be caught once we add the open_channel and accept_channel oracle. So in either case, we don’t need to relax bitcoind default policy to exercise the full funding flow

ekzyis

Only looked at the first commit (5644b83).

I wonder if we also need to unlock the utxos in some cases, like when we fail to broadcast for some reason. Or is that not needed because of snapshot fuzzing, so for every run, bitcoind is also in the same initial state with the utxos unlocked?

But then I wonder why we even need to lock utxos: when would we create two funding transactions with overlapping inputs? Aren't we only creating one funding transaction per run?

erickcestari · 2026-07-06T14:29:39Z

There were a couple of crashes while I was running the funding-flow generator, where we were getting panics in smite because of the restrictive assert statements we have set. These commits fix those issues in smite.

My guess is that a crash can still happens when broadcasting the transaction on chain, because it might try to broadcast the same transaction that was mined before which would make the sendrawtransaction rpc call fail and crash the program.

We could ignore such error and add a special case in the minimizer to remove a BroadcastTransaction operation if it broadcast the same transaction twice, since that would be essentially dead code.

edit: Nevermind, I just saw the second commit that avoid broadcasting the same transaction twice. Which makes a lot of sense, and since is very lightweight operation we maybe don't need to minimize it?

But then I wonder why we even need to lock utxos: when would we create two funding transactions with overlapping inputs? Aren't we only creating one funding transaction per run?

I think smite will be able to open multiple channels against the target. Right now (in current master) Smite cannot open a channel, since it misses the funding and broadcast operations of the open channel flow in the generators.

NishantBansal2003 · 2026-07-06T14:31:39Z

I wonder if we also need to unlock the utxos in some cases, like when we fail to broadcast for some reason. Or is that not needed because of snapshot fuzzing, so for every run, bitcoind is also in the same initial state with the utxos unlocked?

Yep, I think bitcoind will return to the same initial state with a block height of 101, as captured in the snapshot

But then I wonder why we even need to lock utxos: when would we create two funding transactions with overlapping inputs? Aren't we only creating one funding transaction per run?

We have GeneratorInsertionMutator, and eventually have SpliceMutator, which will merge two programs. At that point, we’ll have more than one create funding tx operations

erickcestari · 2026-07-06T14:46:47Z

smite: reject non-relayable funding transactions
Reject funding transactions that would not be accepted
under Bitcoin's default relay policy. Funding construction
now fails for feerates below the minimum relay feerate and
funding outputs below the dust threshold, avoiding
transactions that cannot be relayed or mined by default

Should we care whether the transaction follows the bitcoin core relayable Policy? I think that, if the transaction is consensus-valid, Smite should be able to include it in a block, which would help us cover more edge cases.

NishantBansal2003 · 2026-07-07T17:05:36Z

since is very lightweight operation we maybe don't need to minimize it?

I think any operation that makes redundant or unnecessary RPC calls, should be minimized, since they are time-consuming and actually hurt fuzzing effectiveness

Should we care whether the transaction follows the bitcoin core relayable Policy? I think that, if the transaction is consensus-valid, Smite should be able to include it in a block, which would help us cover more edge cases.

Some reasons I thought against it are:

funding tx below the dust limit or with a very low fee rate will not actually be mined on mainnet
Another thing is that currently the bitcoind backend is shared across smite and the target, so changing the policy could mean the target consults bitcoind to check whether the tx is broadcastable, and bitcoind could say yes. I was thinking of using the default rules for both the target and bitcoind
In the future, we’re going to have an open_channel <-> accept_channel oracle, which will itself flag the case if it isn't rejected by the target, so there is no need to go beyond that point to find those issues for now

But I don’t have a strong opinion, so I’m fine if we want to relax the bitcoind rules instead. Let me know

ekzyis · 2026-07-07T19:00:09Z

funding tx below the dust limit or with a very low fee rate will not actually be mined on mainnet

It could get mined with out-of-band payments, or because it's the miner's own tx etc.

They are non-standard according to Bitcoin Core, but they can get mined.

I agree with @erickcestari, I think never broadcasting txs below the dust limit would unnecessarily limit fuzzing effectiveness, or at least I’m not convinced yet why we shouldn’t broadcast them.

erickcestari · 2026-07-08T13:39:18Z

In the future, we’re going to have an open_channel <-> accept_channel oracle, which will itself flag the case if it isn't rejected by the target, so there is no need to go beyond that point to find those issues for now

I think I got what @NishantBansal2003 said. If a lightning node is building transactions (funding transactions, commitment transactions, HTLC transactions) that aren't accepted by mempool (dust output, nonstandard script, below static minrelay) this is already a bug that should be reported and the fuzzer should crash with help of oracles.

morehouse

The first three commits LGTM. I'd be happy to merge them today as a separate PR.

I'm less sure about the last commit. I agree that we don't want to change the default mempool policy used by the target node, but I think it could be quite profitable to preserve the ability to mine non-standard transactions that may trigger edge cases on the target node.

One way to reconcile the two is to use separate bitcoinds for smite and the target. Of course then the bitcoind nodes communicating with each other would add a new source of latency.

Alternatively, we could keep the shared bitcoind and then if sendrawtransaction fails due to some mempool policy rejection, we could fallback to mining the transaction directly with generateblock instead.

NishantBansal2003 · 2026-07-10T04:27:01Z

Updated the last commit to mine the tx directly if it violates mempool policy while still being consensus-valid in: c193003

morehouse

My main concern is that now the BroadcastTransaction operation may silently mine the transaction as well instead of waiting for MineBlocks to do that.

WDYT about an approach like this?

If the broadcast fails due to non-standardness, the executor saves the raw tx in it's own "private mempool"
On the next MineBlocks operation, if the private mempool contains anything, the executor appends its private mempool to bitcoind's mempool from getrawmempool and mines the combined mempool in the first block with generateblock, then mines the remaining blocks normally.

The fuzzer may build transactions that are consensus-valid but violate mempool policy. `sendrawtransaction` refuses these, so return the raw hex instead of panicking and queue it in a per-executor private mempool, deduplicated on txid. On the next MineBlocks, drain the queue and mine the queued transactions in the first block, including the current mempool so that already-broadcast transactions are not dropped. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

NishantBansal2003 · 2026-07-12T17:14:54Z

My main concern is that now the BroadcastTransaction operation may silently mine the transaction as well instead of waiting for MineBlocks to do that.

WDYT about an approach like this?

If the broadcast fails due to non-standardness, the executor saves the raw tx in it's own "private mempool"

On the next MineBlocks operation, if the private mempool contains anything, the executor appends its private mempool to bitcoind's mempool from getrawmempool and mines the combined mempool in the first block with generateblock, then mines the remaining blocks normally.

Yeah, I think mining blocks only in the MineBlocks operation makes sense for consistency. Now, in daa8ebb, I’ve made sure there’s a clear separation between the two operations based on what they do.

morehouse · 2026-07-17T14:04:31Z

+    /// - If any transaction in `private_mempool` is consensus-invalid.
+    fn mine_block_including(&self, private_mempool: &[String]) {
+        let mut txs = self.get_raw_mempool();
+        txs.extend(private_mempool.iter().cloned());


Nit:

Suggested change

txs.extend(private_mempool.iter().cloned());

txs.extend_from_slice(private_mempool);

morehouse · 2026-07-17T14:07:14Z

+        let address = self.get_new_address();
+        let txs_json = serde_json::to_string(&txs).expect("tx list serializes to valid JSON");


Nit: the txs_json line would fit better in the block above.

morehouse · 2026-07-17T14:09:53Z

+            .arg(&txs_json)
+            .output()
+            .expect("bitcoin-cli generateblock should not fail");
+        assert!(


I think this will panic if there's a duplicate tx or if the list isn't topologically ordered. I think we can't really hit that case currently since the private mempool comes after the public one and we deduplicate on txid in the executor.

But we should still probably call out these scenarios in the "Panics" section above.

morehouse · 2026-07-17T14:39:52Z


    impl BitcoinRpc for MockBitcoinCli {
-        fn mine_blocks(&mut self, num_blocks: u8) {
+        fn mine_blocks(&mut self, num_blocks: u8, _private_mempool: &[String]) {


We should be able to easily mock a mempool rejection to test the new private mempool behavior.

morehouse · 2026-07-17T14:57:04Z

+        // The mempool rejects non-standard or low-feerate transactions. Return
+        // the raw hex so the caller can mine it directly later.
+        if !broadcast_out.status.success() {
+            return Some(signed_tx.hex);
+        }


This considers all failures to be due to non-standardness. I'm not sure if we can get any other kinds of failures currently, but in the future maybe we could?

I suppose we'll get a panic later when mining if the failure here is due to something else. Probably that's okay, though debugging may be slightly trickier in that case.

…policy

ekzyis reviewed Jul 5, 2026

View reviewed changes

NishantBansal2003 force-pushed the bitcoind-fixes branch from e5dc4e7 to 62fbf6c Compare July 7, 2026 17:07

NishantBansal2003 mentioned this pull request Jul 7, 2026

smite-ir: implement funding flow generator #141

Open

morehouse reviewed Jul 8, 2026

View reviewed changes

Comment thread smite/src/bitcoin.rs Outdated

NishantBansal2003 mentioned this pull request Jul 9, 2026

smite: fix funding transaction signing and broadcast #144

Merged

NishantBansal2003 marked this pull request as draft July 9, 2026 07:47

NishantBansal2003 force-pushed the bitcoind-fixes branch from 62fbf6c to c193003 Compare July 10, 2026 04:18

NishantBansal2003 changed the title ~~smite: make funding and broadcast respect bitcoind default policy~~ smite: mine consensus-valid transactions that violate mempool policy Jul 10, 2026

NishantBansal2003 marked this pull request as ready for review July 10, 2026 04:27

morehouse reviewed Jul 10, 2026

View reviewed changes

Comment thread smite/src/bitcoin.rs Outdated

Comment thread smite/src/bitcoin.rs Outdated

Comment thread smite/src/bitcoin.rs Outdated

Comment thread smite/src/bitcoin.rs Outdated

NishantBansal2003 force-pushed the bitcoind-fixes branch from c193003 to daa8ebb Compare July 12, 2026 17:07

NishantBansal2003 requested a review from morehouse July 12, 2026 17:16

morehouse reviewed Jul 17, 2026

View reviewed changes

fixup! smite: mine consensus-valid transactions that violate mempool …

6c4f87d

…policy

NishantBansal2003 force-pushed the bitcoind-fixes branch from 68054f8 to 6c4f87d Compare July 18, 2026 07:38

	txs.extend(private_mempool.iter().cloned());
	txs.extend_from_slice(private_mempool);

		let address = self.get_new_address();
		let txs_json = serde_json::to_string(&txs).expect("tx list serializes to valid JSON");

Conversation

NishantBansal2003 commented Jun 29, 2026

Uh oh!

ekzyis left a comment

Choose a reason for hiding this comment

Uh oh!

erickcestari commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NishantBansal2003 commented Jul 6, 2026

Uh oh!

erickcestari commented Jul 6, 2026

Uh oh!

NishantBansal2003 commented Jul 7, 2026

Uh oh!

ekzyis commented Jul 7, 2026

Uh oh!

erickcestari commented Jul 8, 2026

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NishantBansal2003 commented Jul 10, 2026

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NishantBansal2003 commented Jul 12, 2026

Uh oh!

morehouse Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

erickcestari commented Jul 6, 2026 •

edited

Loading