security: establish offline signer architecture and open source governance - #1
Conversation
Signed-off-by: MaikeH <neomaike@gmail.com>
Signed-off-by: MaikeH <neomaike@gmail.com>
Signed-off-by: MaikeH <neomaike@gmail.com>
Signed-off-by: MaikeH <neomaike@gmail.com>
Signed-off-by: MaikeH <neomaike@gmail.com>
Signed-off-by: MaikeH <neomaike@gmail.com>
neomaike
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/MHX-Digital/cold-wallets/blob/71e00525b4194ff1a9dd12bfe9a9593f43e4976e/start/start.bat#L32
When start.bat is used as documented with an ordinary isolated virtualenv, executing dashboard\server.py makes dashboard/—not the repository root—sys.path[0]. Because the project is not installed as a package and the launcher does not set PYTHONPATH, the first from coordinator.service ... import raises ModuleNotFoundError, so the primary launcher exits before binding its port. Invoke the server as a module or otherwise add the repository root to the import path.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def bitcoin_txid(raw_hex: str) -> str: | ||
| try: raw=bytes.fromhex(raw_hex) | ||
| except ValueError as exc: raise BroadcastStateError("invalid transaction encoding") from exc | ||
| if not raw: raise BroadcastStateError("empty transaction") | ||
| return hashlib.sha256(hashlib.sha256(raw).digest()).digest()[::-1].hex() |
There was a problem hiding this comment.
For every supported native-P2WPKH signed transaction, raw_hex contains the SegWit marker, flag, and witness data, but a Bitcoin txid is the double-SHA256 of the witness-stripped serialization. Hashing the complete payload here instead produces the wtxid, so submit_bitcoin stores the wrong operation identity, will not match the ordinary txid returned by a node, and performs reconciliation lookups under the wrong hash.
Useful? React with 👍 / 👎.
| except TimeoutError as exc: last=TorTimeoutError("Tor request timed out") | ||
| raise last or TorTransportError("Tor request failed closed") |
There was a problem hiding this comment.
When this adapter is backed by the requests.Session constructed in transport/requests_client.py, connect and read timeouts raise requests.exceptions.Timeout, which is not the built-in TimeoutError caught here. Consequently a real timeout escapes after the first request, the configured retry count is ignored, and callers receive an unnormalized requests exception rather than TorTimeoutError.
Useful? React with 👍 / 👎.
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| git diff --check |
There was a problem hiding this comment.
In the checked integrity job, Actions has just produced a clean checkout, so the no-commit form git diff --check only examines uncommitted index/worktree differences and succeeds without inspecting the pushed commit or PR patch. This follows the git diff -h distinction between the worktree form and forms supplied commits; for example, the current target is clean under this command while git diff --check 87ff994^ 87ff994 reports whitespace errors. Compare the event's base and head commits so this gate can actually detect committed whitespace defects.
Useful? React with 👍 / 👎.
1 participant
MERGED EXPERIMENTAL CHECKPOINT — NO-GO FOR REAL FUNDS
This PR promoted the audited separation between a watch-only coordinator,
offline signer, versioned artifact transport, and idempotent broadcaster. It is
an architecture preview, not a production-ready cold wallet.
Delivered
approval.
persistent broadcast state, and Tor-only adapter policy in migrated paths.
changelog, CODEOWNERS, templates, Dependabot, and non-deploy CI.
Verification
71e00525b4194ff1a9dd12bfe9a9593f43e4976e.12ed2fa968d341fcbd12601ff156de884ca3a101.findings.
audit_output/57-c9-checksums.txt: passed.Core, or Docker was used.
Residual risk
Windows-native behavior, real Tor routing, Helios attestation, Bitcoin Core
regtest interoperability, secp256k1 backend provenance/review, and a physical
air gap remain unverified. The product remains NO-GO FOR REAL FUNDS.
Rollback
Use a reviewed revert PR against
main; never rewrite history or restore unsafelegacy entrypoints.
Completed merge gate