smite: add accept_channel oracle by NishantBansal2003 · Pull Request #185 · lnfuzz/smite

NishantBansal2003 · 2026-08-03T09:35:40Z

Add AcceptChannelOracle and consolidate all the previously scattered checks relevant to the initial funding flow negotiation into it, including unknown temporary_channel_id, temporary_channel_id reuse before funding_created, cases where the target accepted an invalid open_channel we sent, and cases where the target sent an accept_channel that does not follow the BOLT 2 open_channel <-> accept_channel requirements

Note: Some checks that depend on negotiated features or channel_type are still missing and will be added in a follow-up PR to keep this one focused
Also, I only added the checks explicitly mentioned in the BOLT 2 and haven't added some obvious checks, such as rejecting max_accepted_htlcs < 1 in open_channel. I do think those checks are valuable, so I was thinking of adding them in a follow up PR to avoid adding too much here. But let me know if you think all of them (excluding feature-related checks) should be included here.

I'm also planning to add BOLT 9 feature flag primitives so feature related logic can be consolidated in one place instead of being scattered across operation.rs, setup.rs, and AcceptChannelOracle. The remaining feature dependent checks will be added to this oracle once that is in place

ekzyis

I was working on using #186 to verify the upfront_shutdown_script TLV and I saw this PR is related. I like the idea of oracles to localize code to detect protocol invariants! I only took a quick look for now. Left some comments regarding the design.

ekzyis · 2026-08-04T16:06:28Z

I see that this file was already defined, but is there a reason why not update the return type of evaluate from OracleResult to Result<(), Violation>?

The documentation of Violation mentions this:

//! Each [`Violation`] variant names a buggy target behavior (e.g., crashing, //! hanging, breaking a protocol invariant). This is how the fuzzer reports bugs //! in the target.

The documentation of oracles.rs also mentions protocol invariants, so it seems like the perfect match to me. I think this would make the evaluate_accept_channel_oracle function obsolete.

Not sure how useful this would be, but we could then even consider oracles to return all violations instead of only the first one.

I don't have a strong opinion here and can certainly change OracleResult to Result<(), Violation>, though I do prefer a separate block for oracle evaluation and hence would like to keep evaluate_accept_channel_oracle

Not sure how useful this would be, but we could then even consider oracles to return all violations instead of only the first one.

How will this work? Do you mean each oracle failure is only logged, or we store each oracle fail in a Vec and then output it?

I also think Result<(), Violation> would be simpler.

I would rather not have oracles return multiple violations though -- it's simpler to just return the first one. Realistically I would be surprised (in a bad way) if multiple violations are common, and even then we should be able to find the next violation after the first one is fixed.

ekzyis · 2026-08-04T16:20:54Z

-    // The opener cannot afford the fee, so the acceptor must not send
-    // `funding_signed`. Receiving one is a protocol violation.
-    if !state.config.can_opener_afford_feerate(&state.commitment) {
-        return Err(Violation::OpenerCannotAffordFee(fs.channel_id));
-    }
-


verify_funding_signed can still throw a Violation. Is the plan to replace verify_funding_signed with an oracle? I expected all Violations to be thrown in oracles.

Yup, that is originally my next plan to migrate those to oracles as well

ekzyis · 2026-08-04T16:29:19Z

+/// Panics if no matching `open_channel` exists. This should be unreachable, as
+/// `evaluate_accept_channel_oracle` reports such messages as a [`Violation`].


Mhh, would be nice if we could couple oracle evaluation ("verification") with receiving or recording.

I think conceptually, verification is closer to receiving. If we would move it there, we would still need to document a possible panic here, but if the only function that receives also verifies, and we use naming conventions like recv_<message> and record_<message> for the functions we call in the matching arm of Executor::execute(), it would be easier to parse the code and have confidence in no panics.

(Or we move verification to recording and then we don't need to panic.)

I do prefer all three to be separate, we already follow the same idea currently with funding_signed -- receive and then verify instead of clubbing them together (for logging purposes as well)

I can certainly club verify with record, but I actually wanted to keep the oracle evaluation blocks separate as // -- Oracle evaluation --, so we can put all the oracle evaluation there, which is easier to understand. Then we can follow a pattern like recv -> verify oracle -> record

morehouse

My main concern is that we might get false positives for channel types we haven't implemented yet (i.e. taproot, zero-fee commits). Please double check that and add comments explaining why we're safe (if we are).

Otherwise this is great -- I have only minor comments inline.

Also things that we should consider adding in follow-up PRs:

`open_channel` expected failures

zero_fee_commitments && max_accepted_htlcs > 114
zero_fee_commitments && feerate_per_kw != 0
wumbo limit exceeded
maybe feerate_per_kw == 0?
maybe channel_reserve_satoshis >= funding_satoshis
maybe chain_hash != regtest if we add non-regtest chain hashes to the IR
maybe announce_channel && option_scid_alias
maybe absurd dust_limit_satoshis values (e.g., more than 10,000 sat?)

`accept_channel` violations

channel_type != open_channel.channel_type
option_zeroconf && minimum_depth > 0
invalid upfront_shutdown_script
zero_fee_commitments && max_accepted_htlcs > 114
maybe when any pubkeys match a pubkey from open_channel
maybe per_commitment_point reuse -- requires maintaining a global PCP set. But we'll want to maintain that set anyway for commitment fuzzing
maybe to_self_delay == 0
maybe htlc_minimum_msat > max_htlc_value_in_flight_msat
maybe htlc_minimum_msat > open_channel.funding_satoshis
maybe max_accepted_channels == 0

morehouse · 2026-08-06T19:10:50Z

+    // Check that the channel type was included.
+    let Some(channel_type) = open_channel.tlvs.channel_type.as_deref() else {
+        return Some("open_channel does not include a channel_type".to_string());
+    };


I know the spec says the receiver must fail in this case, but I wouldn't be surprised if there's still an implementation that allows the old "implicit" negotiation...

I think then that's a non-spec-compliant bug, I will add a TODO to check option_channel_type in the negotiated features since it is assumed to be supported

Required when moving fee checks to accept_channel oracle where we wouldn't need to construct the whole channel state just to compute the cost. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

Define `AcceptChannelOracle` and migrate existing `accept_channel` validation into it, consolidating BOLT 2 v1 channel establishment checks: - verify `temporary_channel_id` maps to a sent `open_channel` - validate funding amounts and channel type presence - check the opener can afford the proposed commitment feerate - detect reused `temporary_channel_id`s before `funding_created` The oracle brings together validation that was previously spread across `record_recv_accept_channel()`, `verify_funding_signed()` and implicit checks. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

NishantBansal2003 · 2026-08-07T20:30:25Z

Had to force-push since there were some design changes

My main concern is that we might get false positives for channel types we haven't implemented yet (i.e. taproot, zero-fee commits). Please double check that and add comments explaining why we're safe (if we are).

I verified this and added comments inline. For the fee and anchor cost calculations, we might be undercalculating the fee for 0FC or taproot channels, but definitely not overcalculating it. So we will not get false positives, though we might miss some invalid cases. In some cases, we might also hit a Violation where the error string is misleading, but it will still be an actual error.

I think this will also be resolved once I add support for verifying oracles based on the negotiated features in a follow-up PR.

morehouse

Ready to squash the fixup.

morehouse · 2026-08-07T21:31:55Z

+
+/// Verifies that the initial commitment can cover its fee and satisfies the
+/// channel reserve requirement, returning the first requirement it breaches,
+/// or None if both are met.


Doesn't return None.

morehouse · 2026-08-07T21:32:17Z

+}
+
+/// Verifies the `accept_channel` against the BOLT 2 requirements it must meet,
+/// returning the one it breaches, or `None` if it meets them all.


Doesn't return None.

morehouse · 2026-08-07T21:32:27Z

+}
+
+/// Returns the BOLT 2 requirement our `open_channel` breaches, i.e. the reason
+/// its receiver had to fail the channel instead of accepting it, or `None` if


Doesn't return None.

morehouse · 2026-08-07T21:32:57Z

+    /// Returns `Violation` if an oracle invariant is violated.
+    fn evaluate(&self, context: &C) -> Result<(), Violation>;
    /// Return the name of this oracle for logging
    fn name(&self) -> &str;


Looks like we can delete name entirely.

morehouse · 2026-08-07T21:34:21Z

+const MAX_ACCEPTED_HTLCS_LIMIT: u16 = 483;
+const MIN_DUST_LIMIT_SATOSHIS: u64 = 354;
+
+/// `AcceptChannelContext` is the context for the `AcceptChannelOracle`


Nit

Suggested change

/// `AcceptChannelContext` is the context for the `AcceptChannelOracle`

/// Context for `AcceptChannelOracle`

morehouse · 2026-08-07T21:34:45Z

+    pub negotiation: Option<&'a PendingChannel>,
+}
+
+/// `AcceptChannelOracle` checks whether the `open_channel` answered by an


Nit

Suggested change

/// `AcceptChannelOracle` checks whether the `open_channel` answered by an

/// Checks whether the `open_channel` answered by an

morehouse · 2026-08-07T21:37:52Z


    #[test]
-    fn can_opener_afford_feerate_checks() {
+    fn opener_balance_after_fee_sat_checks() {


This test name no longer matches the API.

ekzyis reviewed Aug 4, 2026

View reviewed changes

morehouse reviewed Aug 6, 2026

View reviewed changes

NishantBansal2003 added 4 commits August 8, 2026 00:47

smite: extract commitment cost into CommitmentCost struct

3c6435b

Required when moving fee checks to accept_channel oracle where we wouldn't need to construct the whole channel state just to compute the cost. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

smite: validate open_channel in accept_channel oracle

10565c8

Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

smite: validate accept_channel in accept_channel oracle

3fea27c

Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

NishantBansal2003 force-pushed the accept-chan-oracle branch from 5eca139 to 3fea27c Compare August 7, 2026 20:06

fixup! smite: extract commitment cost into CommitmentCost struct

f67453c

morehouse approved these changes Aug 7, 2026

View reviewed changes

		/// Panics if no matching `open_channel` exists. This should be unreachable, as
		/// `evaluate_accept_channel_oracle` reports such messages as a [`Violation`].

	/// `AcceptChannelContext` is the context for the `AcceptChannelOracle`
	/// Context for `AcceptChannelOracle`

	/// `AcceptChannelOracle` checks whether the `open_channel` answered by an
	/// Checks whether the `open_channel` answered by an

Uh oh!

Conversation

NishantBansal2003 commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ekzyis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ekzyis Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

open_channel expected failures

accept_channel violations

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NishantBansal2003 commented Aug 7, 2026

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NishantBansal2003 commented Aug 3, 2026 •

edited

Loading

ekzyis Aug 4, 2026 •

edited

Loading

`open_channel` expected failures

`accept_channel` violations