Update documentation using deterministic generation#8557
Conversation
ShahanaFarooqui
left a comment
There was a problem hiding this comment.
@rustyrussell The changes in ./tests/autogenerate-rpc-examples.py look good at first glance, but I am unable to test them locally due to missing test data files:
./tests/data/autogenerate-bitcoin-blocks.json
./tests/data/autogenerate-bitcoind-wallet.dat
Could you please add these files or provide instructions on how to generate them for local testing?
9fc3b5e to
c455c51
Compare
|
Now And |
9d0aae6 to
43fdc51
Compare
Unless REGENERATE_BLOCKCHAIN is true, in which case we make them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes our transaction generation (nlocktime) consistent, as well as our htlcs for payment. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Move the elements config writing into an ElementsD.set_port() override. This makes many of our information outputs consistent. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Don't mangle bookkeeper events. 2. Use l3, not l1, for listtransactions: it's more stable. 3. Use l2, not l1, for listpeerchannels and listchannels. 3. Use l2, not l2, for listfunds. (l1 has a lot of variation). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And hand in CLN_NEXT_VERSION to replace the version string. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…s_settled, wait_for_wallet_txs
… request in Rewriter
…smtool getsecret`
…stabilise the response And flake fixes
…wProxy mocks These tests built their RawProxy against a hardcoded bitcoin.conf in the node's datadir. That only worked on liquid-regtest by accident: BitcoinD.__init__ used to write bitcoin.conf even when the node was actually an ElementsD. Now that config writing is deferred to set_port() and ElementsD only writes elements.conf, the mocks found no config and no .cookie, so their RPC calls failed and bcli's getblock timed out, leaving lightningd stuck during restart. Use bitcoind.conf_file, which points at the config the running daemon actually uses (elements.conf on liquid-regtest, bitcoin.conf on egtest).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
a8eafc8 to
00dc521
Compare
4aae26d
into
ElementsProject:master
| - name: Debug check-doc-examples inputs | ||
| run: | | ||
| echo "== lightning-hsmtool ==" | ||
| which lightning-hsmtool || echo "NOT ON PATH" | ||
| find . -name "lightning-hsmtool*" -not -path "./.git/*" 2>/dev/null | ||
|
|
||
| echo "== autogenerate-bitcoind-wallet.dat ==" | ||
| ls -la tests/data/autogenerate-bitcoind-wallet.dat 2>&1 | ||
|
|
||
| echo "== autogenerate-bitcoin-blocks.json ==" | ||
| ls -la tests/data/autogenerate-bitcoin-blocks.json 2>&1 | ||
| echo "-- number of blocks --" | ||
| python3 -c "import json; print(len(json.load(open('tests/data/autogenerate-bitcoin-blocks.json'))))" 2>&1 | ||
| echo "-- first 300 chars --" | ||
| head -c 300 tests/data/autogenerate-bitcoin-blocks.json | ||
| echo "" | ||
|
|
||
| echo "== environment fingerprint ==" | ||
| # Determinism discriminators: HAVE_USDT toggles tracing, which | ||
| # must not perturb the deterministic RNG stream. | ||
| grep -E 'HAVE_USDT|HAVE_SQLITE3' config.vars || true | ||
| bitcoind --version | head -1 || true | ||
| dpkg -l systemtap-sdt-dev 2>/dev/null | tail -1 || true | ||
|
|
There was a problem hiding this comment.
Don't forget to remove this debug scaffolding.
| executable=None, | ||
| bad_notifications=False, | ||
| old_hsmsecret=None, | ||
| no_entropy=False, |
There was a problem hiding this comment.
The naming seems backwards. no_entropy=True actually enables the deterministic entropy seed (CLN_DEV_ENTROPY_SEED), so the name reads as the opposite of what it does. Consider deterministic_entropy= or entropy_seed=, or even add a docstring line here clarifying "no real entropy -> use a fixed dev seed."
| if no_entropy: | ||
| self.daemon.env["CLN_DEV_ENTROPY_SEED"] = str(node_id) |
There was a problem hiding this comment.
Looking at the behaviour makes it clear the variable does the opposite.
| if base_port: | ||
| port = base_port + node_id * 2 - 1 | ||
| grpc_port = base_port + node_id * 2 |
There was a problem hiding this comment.
With base_port set, port/grpc_port are computed deterministically and intentionally not added to reserved_ports (line 1876). That's fine for a single serial doc-gen run, but a collision here surfaces as a confusing bind error instead of a clean reservation retry. Worth a one-line comment stating the assumption (single serial run over a fixed range).
| if not base_port: | ||
| self.reserved_ports.append(port) |
There was a problem hiding this comment.
base_port ports aren't reserved or collision-checked.
| bool randbytes_overridden(void) | ||
| { | ||
| return dev_seed != 0; | ||
| } |
There was a problem hiding this comment.
Pre-existing, but flagging since determinism is the whole point of this PR: randbytes_overridden() keys off dev_seed != 0, and the basename change alters the siphash24 input. If the hash ever lands on exactly 0, the override silently disables and output becomes non-deterministic rather than erroring. A dedicated dev_overridden bool would be more robust than sentinel-zero.
| /* Hash only the basename: binaries still get distinct seeds, but | ||
| * the stream no longer depends on the checkout path. Plugins and | ||
| * subdaemons are exec'd with absolute paths: hash only the basename, | ||
| * so the stream doesn't depend on where the source tree lives */ |
There was a problem hiding this comment.
nit: the comment says "hash only the basename" twice (lines 59 and 61–62 restate it), and lines 60/62 are indented one space short of the block (\t* vs \t *).
Depends on #8556Merged!Now we do major surgery on autogenerate-rpc-examples.py. Finally we run it to produce the canned blocks and wallet for future runs, and re-enable it in CI.