Skip to content

fix: write committee instance keys durably next to the database and fail fast on bad startup settings - #539

Open
KSlashh wants to merge 1 commit into
devfrom
audit/36-key-and-startup-safety
Open

KSlashh wants to merge 1 commit into
devfrom
audit/36-key-and-startup-safety

Conversation

@KSlashh

@KSlashh KSlashh commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Audit items L17, L66, L59, L69, L70, L71. Open against dev.

Instance key file (L17). The envelope was written in place with no fsync, and its 0600 mode was set after the write. The committee answers the peg-in on chain right after this, and the file is the only copy of the key. A power loss could leave an empty or partial file. Now the envelope goes to a 0600 temporary file in the same directory, which is synced, renamed into place, and the directory is synced too. After a crash the file is either complete or absent. A temporary file left by an interrupted write is replaced.

Key directory (L66). The directory was cache/committee-instance-keys/, relative to the working directory. A node restarted from another directory no longer found its keys and stopped signing for open instances. In the Docker image the working directory is /app, outside the /var/data volume, so recreating the container lost every instance key. Now:

  • The directory is cache/committee-instance-keys next to the database file, or COMMITTEE_INSTANCE_KEYS_DIR. It is resolved to an absolute path once at startup, before the engine starts, and logged.

  • A committee node checks at startup that it can write there.

  • The run scripts and the localnet keep the database in the working directory, so their key path does not change. Docker nodes move to /var/data/cache/committee-instance-keys.

Rollback note (L59). The deterministic nonce derivation now documents its precondition: the record of what the node signed must never be rolled back. A MuSig2 partial signature also depends on the other signers' nonces, so after a rollback a co-signer can make the node sign again with the same secret nonce. The deployment guide says the same about the database and the key directory.

External address (L69). The operator stored its STUN-discovered address with set_var while other threads were already running, which is undefined behavior on glibc. The address is now kept in a process-wide cell that get_local_node_info reads. EXTERNAL_SOCKET_ADDR still wins.

Startup check (L70). check_node_info called Goat with no timeout or retry and panicked on any error, so a stuck connection hung startup. The two Goat reads now go through the existing boot retry: each call is bounded by the Goat RPC timeout and retried for about a minute. Failures are returned as errors. Operators no longer make the unused Goat call.

Network names (L71).

  • An unknown BITCOIN_NETWORK or GOAT_NETWORK stops the node at startup instead of falling back to testnet4 or test.

  • pegin-request parses --network the same way and refuses a value that disagrees with BITCOIN_NETWORK, so addresses are no longer checked against two networks. It no longer accepts testnet (testnet3), which the node does not support.

Deployment. A Docker committee node that holds keys for open instances must copy /app/cache/committee-instance-keys from the old container to /var/data/cache/committee-instance-keys before it is recreated, or set COMMITTEE_INSTANCE_KEYS_DIR to the old path.

Not changed. A deletion job that finds no envelope still records the key as deleted. With the directory fixed at startup, a missing file means there was no key. Keeping the record open would make the deletion run again on every block.

Tests: the envelope is written with 0600 and a leftover temporary file is replaced; the key directory follows each database path form and the environment override; an in-memory database needs the override; unknown network names are rejected. bitvm-gc and node tests, clippy with -D warnings (default and mock-gc) and fmt pass.

The test coverage and validation notes above are from the zkm PR notes. Branch scope and diff formatting were checked locally; builds and tests were not rerun during publication.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant