smite-scenarios: sync target chain view via RPC after mining by NishantBansal2003 · Pull Request #211 · lnfuzz/smite

NishantBansal2003 · 2026-08-19T16:54:22Z

Targets (CLN, LDK) previously learned about new blocks through bitcoind's blocknotify, which fires asynchronously. When multiple blocks are mined, this can queue up many sync calls, adding unnecessary load. Instead, explicitly sync the target once after each MineBlocks operation.
Introduce a TargetRpc trait with a chain_sync() method, called by the executor immediately after MineBlocks. CLN and LDK sync explicitly via RPC and signals, respectively, while LND and Eclair use ZMQ and remain no-ops.
Dropping -blocknotify also removes the burst of asynchronous SIGUSR1s during initial block generation, so LDK no longer needs the pre_exec SIG_IGN

The reason I went with an RPC trait is that, in the future, we'll need access to the target's CLI/RPC to add payment hash and preimage information to the target so that it can send an update_fulfill_htlc message.
Also, for now, I only added the CLI as a trait implementation for each target to keep the changes contained and concise. In the future, we can definitely add direct RPC calls over the socket as well to save overhead.

erickcestari

Good idea! It's much better that it only calls the sync signal once per MineBlock instead of calling it for every block mined.

erickcestari · 2026-08-19T17:33:17Z

+        assert!(
+            out.status.success(),
+            "pkill -USR1 ldk-node-wrapper failed: {}",
+            String::from_utf8_lossy(&out.stderr)
+        );


Should we have an assert here? Imagine the executor crashes the target while executing the input. It would probably fail to sync and panic with a pkill error instead of failing with a Violation::Crashed error.

Hmm, great point, I think pkill or lightning-cli should always succeed in any case, and they should only error out when the target has already crashed. So, I think we could check whether the target has crashed before asserting this. If it has crashed, we can simply log the failure here, otherwise, we should assert it. I don't think there is any other case where RPC commands should be unresponsive, so I think it's better to be stricter. WDYT?

Yes, I agree! I also think logging is a good option.

erickcestari · 2026-08-19T17:34:57Z

            cmd.env("LD_PRELOAD", handler);
        }

-        // Ignore SIGUSR1 for the window between exec and the wrapper blocking


Some comments at workloads/ldk/src/main.rs also need to be updated after the remove of SIG_IGN.

NishantBansal2003 · 2026-08-25T13:26:54Z

I updated the CLN lightning-cli based calls to use direct RPC calls instead. This gives us more flexibility in detecting when someone is listening on the Unix RPC socket, allowing us to handle that case explicitly (as discussed in #211 (comment)) while hard-asserting on any other errors. The approach is largely inspired by https://github.com/rust-bitcoin/corepc/blob/8d82c88aebf8268bfc9732aaa15e64a5bba1f9a0/jsonrpc/src/simple_uds.rs and the cln_rpc rust crate.

I verified this by crashing the target before syncing the chain. CLN and LDK are now able to distinguish between the target having already crashed and other errors, logging the former and hard-asserting on the latter.

I haven't done the same for LND and Eclair since they currently don't use any CLI calls. I'll address those in follow-up PRs, as we need to make RPC calls for them as well

erickcestari

LGTM! I think we can optimize this a bit by avoiding spawning the pkill process, consequently reducing the memory overhead and the amount of memory dirtied by process creation.

erickcestari · 2026-08-31T13:09:11Z

+    fn chain_sync(&mut self) {
+        let out = Command::new("pkill")
+            .arg("-USR1")
+            .arg("-f")
+            .arg("^ldk-node-wrapper")
+            .output()
+            .expect("pkill -USR1 ldk-node-wrapper should not fail");
+
+        match out.status.code() {
+            Some(0) => {}
+            // pkill exits 1 when nothing matches, so LDK is gone, check_alive
+            // will report the crash at the end.
+            Some(1) => log::warn!("ldk-node-wrapper is not running, skipping chain sync"),
+            _ => panic!(
+                "pkill -USR1 ldk-node-wrapper failed: {}",
+                String::from_utf8_lossy(&out.stderr)
+            ),
+        }
+    }


We could use the ldk ManagedProcess to get the PID and use the syscall kill directly instead of creating a pkill process to do this for us.

Done and Rebased!

Targets previously learned about new blocks through bitcoind's blocknotify, which fires asynchronously. When multiple blocks are mined, this can queue up many sync calls, adding unnecessary load. Instead, explicitly sync the target once after each MineBlocks operation. Introduce a TargetRpc trait with a chain_sync() method, called by the executor immediately after MineBlocks. CLN and LDK sync explicitly via RPC and signals, respectively, while LND and Eclair use ZMQ and remain no-ops. Dropping -blocknotify also removes the burst of asynchronous SIGUSR1s during initial block generation, so LDK no longer needs the pre_exec SIG_IGN. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>

morehouse · 2026-09-03T20:24:54Z

+        assert!(
+            response.get("error").is_none(),
+            "lightningd rejected {method}: {}",
+            response["error"]
+        );
+
+        Ok(response["result"].take())


Should we define an RpcResponse struct like we do for all other RPCs? It would make it clearer what we expect a response to contain.

morehouse · 2026-09-03T20:25:07Z

+#[derive(Debug, Clone)]
+pub struct ClnRpc {
+    /// Path to the CLN node's unix RPC socket.
+    pub rpc_socket: PathBuf,


Suggested change

pub rpc_socket: PathBuf,

rpc_socket: PathBuf,

morehouse · 2026-09-03T20:35:42Z

+}
+
+impl ClnRpc {
+    // Bound RPC socket I/O so a stalled lightningd cannot block indefinitely.


Suggested change

// Bound RPC socket I/O so a stalled lightningd cannot block indefinitely.

/// Bound RPC socket I/O so a stalled lightningd cannot block indefinitely.

morehouse · 2026-09-03T21:19:40Z

+        let mut response: serde_json::Value = serde_json::Deserializer::from_reader(&mut sock)
+            .into_iter()
+            .next()
+            .unwrap_or_else(|| panic!("lightningd closed the socket without answering {method}"))
+            .unwrap_or_else(|e| panic!("failed to read {method} response from lightningd: {e}"));


This code is confusing -- what is being unwrapped twice? Also, the second panic triggers on timeouts too, which seems like it should also be "warn-only" since a crash/hang will be detected later.

Perhaps we should match on the response for clarity and to let us distinguish the timeout error types from other types.

I updated the error handling to cover possible cases where the target is hung or crashed, whether before the call, during the write, or during the read, and also made it a bit clearer. I also verified some crash cases by crashing CLN before the run, after the write, and after the read, and the handling covers all of them

Wasn't fully sure how to test the hang cases, but I think TimedOut or UnexpectedEof should cover those cases as well

morehouse · 2026-09-03T21:30:05Z

+/// defaults to terminating the process. That distinction is the whole point: an
+/// *ignored* signal is discarded the moment it is delivered, while a *blocked*
+/// one stays pending until `sigwait()` consumes it, regardless of its
+/// disposition. So this call is what makes target's SIGUSR1 observable, and
+/// why nothing here calls `sigaction`: the wait loop below is the only consumer
+/// these signals need.


No need to explain the difference between signal-blocking and SIG_IGN anymore.

Suggested change

/// defaults to terminating the process. That distinction is the whole point: an

/// *ignored* signal is discarded the moment it is delivered, while a *blocked*

/// one stays pending until `sigwait()` consumes it, regardless of its

/// disposition. So this call is what makes target's SIGUSR1 observable, and

/// why nothing here calls `sigaction`: the wait loop below is the only consumer

/// these signals need.

/// defaults to terminating the process. A blocked SIGUSR1

/// stays pending until `sigwait()` consumes it, regardless of its

/// disposition. So this call is what makes target's SIGUSR1 observable, and

/// why nothing here calls `sigaction`: the wait loop below is the only consumer

/// these signals need.

morehouse · 2026-09-03T21:37:59Z

+            panic!(
+                "failed to send SIGUSR1 to ldk-node-wrapper (pid {}): {e}",
+                self.pid
+            );


Why do we panic for LDK but warn for CLN in the same situation?

Because bad RPC calls already panic inside self.run for CLN, while cases where CLN has already crashed or hung just return Err so the caller can log them. In the future, I think we'll mostly remove this pattern, and will return Err directly from chain_sync or other RPC calls and instead let ir.rs log the RPC error itself, since we may also run commands like addinvoice and want their responses to proceed further

For LDK, the signal-based approach is essentially fire-and-forget. Even if LDK has crashed or hung, its PID remains until the parent reaps it, so the signal is delivered normally (though it won't be responsive) and the call returns Ok. So, any other error is unexpected, so we panic on it

ekzyis · 2026-09-04T16:30:10Z

+impl TargetRpc for LndRpc {
+    /// LND receives new blocks directly from bitcoind over ZMQ, so no manual
+    /// chain synchronization is required.
+    fn chain_sync(&mut self) {}


nit: could it make sense to assert the block height here, since after calling this function, I think the caller expects the chain to be synced? But then we would need to pass the expected block height somehow.

Yeah, I mean that is the defensive way, but the current sync model is essentially fire-and-forget. To actually get the updated block height for each implementation, we would need to wait for some time and then make another RPC call, and I'm not sure how long we should wait or how many times we should retry. In the worst case, we might make several RPC calls, which could hurt the fuzzing itself

So I believe a cheaper way to check whether the blocks have been synced is to run a full funding flow and see if we receive channel_ready from the peer

erickcestari suggested changes Aug 19, 2026

View reviewed changes

NishantBansal2003 force-pushed the target-cli branch from 81c6746 to 879486b Compare August 25, 2026 13:18

NishantBansal2003 requested a review from erickcestari August 25, 2026 13:26

NishantBansal2003 commented Aug 25, 2026

View reviewed changes

Comment thread smite-scenarios/src/targets/cln.rs Outdated

NishantBansal2003 mentioned this pull request Aug 27, 2026

Add ReorgChain IR operation #219

Open

NishantBansal2003 force-pushed the target-cli branch from 879486b to 9578d23 Compare August 31, 2026 07:10

erickcestari suggested changes Aug 31, 2026

View reviewed changes

NishantBansal2003 force-pushed the target-cli branch 2 times, most recently from 5931f69 to 9975e59 Compare September 2, 2026 08:59

NishantBansal2003 force-pushed the target-cli branch from 9975e59 to b3a753b Compare September 2, 2026 09:01

morehouse reviewed Sep 3, 2026

View reviewed changes

ekzyis reviewed Sep 4, 2026

View reviewed changes

fixup! smite-scenarios: sync target chain view via RPC after mining

4529f5a

	// Bound RPC socket I/O so a stalled lightningd cannot block indefinitely.
	/// Bound RPC socket I/O so a stalled lightningd cannot block indefinitely.

Uh oh!

Conversation

NishantBansal2003 commented Aug 19, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NishantBansal2003 Aug 20, 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!

NishantBansal2003 commented Aug 25, 2026

Uh oh!

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

erickcestari Aug 31, 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!

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!

4 participants

NishantBansal2003 Aug 20, 2026 •

edited

Loading

erickcestari Aug 31, 2026 •

edited

Loading