bitcoind: replace bitcoin-cli with a keep-alive JSON-RPC client

[erickcestari]

Every bitcoind call from the fuzz loop forked a bitcoin-cli process inside the Nyx VM; a full funding flow spawned 12 of them. This replaces the transport with a small HTTP/1.1 client over one TcpStream to bitcoind's JSON-RPC port, then moves the startup calls onto it too so bitcoin-cli disappears from the images.

Benchmark

input:

smitebot print-ir ./input.bin
v0 = LoadChainHashFromContext()
v1 = LoadChannelId(0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb)
v2 = LoadAmount(100000)
v3 = LoadAmount(0)
v4 = LoadAmount(546)
v5 = LoadAmount(100000000)
v6 = LoadAmount(1000)
v7 = LoadAmount(1000)
v8 = LoadFeeratePerKw(2500)
v9 = LoadU16(144)
v10 = LoadU16(483)
v11 = LoadPrivateKey(0x2121212121212121212121212121212121212121212121212121212121212121)
v12 = DerivePoint(v11)
v13 = LoadPrivateKey(0x2222222222222222222222222222222222222222222222222222222222222222)
v14 = DerivePoint(v13)
v15 = LoadPrivateKey(0x2323232323232323232323232323232323232323232323232323232323232323)
v16 = DerivePoint(v15)
v17 = LoadPrivateKey(0x2424242424242424242424242424242424242424242424242424242424242424)
v18 = DerivePoint(v17)
v19 = LoadPrivateKey(0x2525252525252525252525252525252525252525252525252525252525252525)
v20 = DerivePoint(v19)
v21 = LoadPrivateKey(0x2626262626262626262626262626262626262626262626262626262626262626)
v22 = DerivePoint(v21)
v23 = LoadU8(0)
v24 = LoadShutdownScript(Empty)
v25 = LoadChannelType(StaticRemoteKey)
v26 = BuildOpenChannel(v0, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v12, v14, v16, v18, v20, v22, v23, v24, v25)
v27 = SendOpenChannel(v26)
v28 = RecvAcceptChannel(v27)
v29 = ExtractFundingPubkey(v28)
v30 = CreateFundingTransaction(v12, v29, v2, v8)
v31 = SendFundingCreated(v30, v11, v1)
v32 = RecvFundingSigned(v31)
BroadcastTransaction(v30)
MineBlocks(6)
v35 = LoadPrivateKey(0x2727272727272727272727272727272727272727272727272727272727272727)
v36 = DerivePoint(v35)
v37 = LookupShortChannelId(v30)
SendChannelReady{include_alias=false}(v32, v36, v37)
RecvChannelReady()

master:

  aggregate over 3 runs:
    execs/sec: mean 13.8  stddev 0.3  min 13.5  max 14.1
    boot + snapshot: mean 3.935 s
    latency (mean across runs):
      min 65.18 ms  mean 72.71 ms  median 73.82 ms  p99 78.82 ms  max 79.84 ms
    input execution (mean across runs): mean 70.41 ms  median 71.53 ms
    nyx overhead (mean across runs):     mean 2.30 ms  median 2.48 ms
    failed iterations: 0 / 600
    coverage determinism: 92.5% stable (4129 / 55086 edges fluctuated, summed over runs)

This pr:

  aggregate over 3 runs:
    execs/sec: mean 15.0  stddev 0.0  min 14.9  max 15.0
    boot + snapshot: mean 3.893 s
    latency (mean across runs):
      min 59.04 ms  mean 66.82 ms  median 66.70 ms  p99 69.81 ms  max 72.00 ms
    input execution (mean across runs): mean 64.40 ms  median 64.28 ms
    nyx overhead (mean across runs):     mean 2.42 ms  median 2.40 ms
    failed iterations: 0 / 600
    coverage determinism: 74.7% stable (13998 / 55302 edges fluctuated, summed over runs)

About 10% on this input. Inputs with fewer bitcoind calls gain proportionally less.

[NishantBansal2003]

Have you considered using the jsonrpc crate's simple_http transport? It would replace most of the hand-written HTTP code while providing keep-alive connections, reconnects, basic auth, and request timeouts. It's fully synchronous and doesn't spawn processes, so it fits the goals of this PR with less code

It does add a dependency, but it’s a small one. I think that's a good trade-off for the manual HTTP handling it removes. For reference, electrs uses the same setup: https://github.com/romanz/electrs/blob/d975dd03f2a410c4473116d5a2be950a205e0443/Cargo.toml#L31

[erickcestari]

Have you considered using the jsonrpc crate's simple_http transport? It would replace most of the hand-written HTTP code while providing keep-alive connections, reconnects, basic auth, and request timeouts. It's fully synchronous and doesn't spawn processes, so it fits the goals of this PR with less code

It does add a dependency, but it’s a small one. I think that's a good trade-off for the manual HTTP handling it removes. For reference, electrs uses the same setup: https://github.com/romanz/electrs/blob/d975dd03f2a410c4473116d5a2be950a205e0443/Cargo.toml#L31

I think it's better to avoid adding a dependency for ~120 lines of code, and also keep control over the implementation.

[ekzyis]

I think CLN requires bitcoin-cli here. lightningd fails to sync with a898cf8:

INFO  [smite_scenarios::targets::bitcoind] Waiting for bitcoind to be ready...
INFO  [smite_scenarios::targets::bitcoind] bitcoind is ready
INFO  [smite_scenarios::targets::cln] Starting lightningd...
INFO  [smite_scenarios::targets::cln] Waiting for lightningd to be ready and synced...
DEBUG [smite::process] bitcoind: dropping running process, attempting shutdown
DEBUG [smite::process] bitcoind: sending SIGTERM to process group 149
DEBUG [smite::process] bitcoind: exited with exit status: 0
ERROR [smite::scenarios] Failed to initialize scenario: target error: failed to start: lightningd failed to sync chain

[-] PROGRAM ABORT : Something went wrong ...
         Location : afl_fsrv_start(), src/afl-forkserver.c:835

It works on 40e7f1f. Can you reproduce?

[ekzyis]

nit: I think this was already wrong. I think mine_blocks doesn't panic anymore on 0 since a22c79b.