Release: develop -> main - #10
Merged
Merged
Conversation
The bitcoin crate's script parser returns Instruction::PushBytes(&[]) for 0x00 (OP_FALSE/OP_PUSHBYTES_0), not Instruction::Op(OP_FALSE). This caused the OP_FALSE OP_IF envelope detection to never trigger, so inscription content was never extracted.
- Add atomic_write helper (write to .tmp, fsync, rename) to prevent data corruption on process crash - Apply atomic writes to accounts, state, and block hash files - Replace panicking .unwrap() with recoverable error handling in account_server (mutex lock, proof deserialization)
Reject duplicate coins in receive_coin: check both the pending coin_queue and the spent coin_history before accepting a coin.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Verify sender identity via Schnorr signature over request fields (address, recipient, amount, timestamp). Timestamps older than 5 minutes are rejected. Backwards compatible: signature is optional, allowing gradual client migration.
…ockerfile description
AccountState::new() used random bytes as blinding factor for the address: hash(public_key || random_bytes). This made the same mnemonic produce different addresses on each call, breaking seed phrase recovery and passkey re-authentication completely. Fix: address = hash(public_key) — deterministic, same mnemonic always produces the same address. Privacy is provided by ZK proofs, not by address blinding. - Update MINTING_ADDRESS constant to match new derivation - Rebuild ELF binary for the updated ZK circuit - Update test helpers to use deterministic address computation - All 13 tests pass
Use ClientAccount::new() instead of hash(xpriv_string) for the minting account address. Add assertion that derived address matches the MINTING_ADDRESS constant to catch mismatches early.
…tions The publisher serialized Option<Commitment> (with bincode variant tag) but the scanner deserialized as Commitment (without). This 1-byte offset caused "malformed public key" errors, preventing commitments from being added to the SMT and breaking all subsequent operations.
send_coins previously used take() on coin_queue and proof before the prover call. If the prover or merkle proof lookup failed, the account permanently lost its proof and pending coins. Fix: read coin_queue and proof non-destructively (clone/iter), only commit state changes (clear queue, update proof/balance) after the prover succeeds. This prevents account corruption when a second mint fails because the first inscription hasn't been confirmed on Bitcoin yet.
…ve sends send_coins used the CURRENT public key to look up the previous commitment in the SMT, but commitments are indexed by the key that SIGNED them (the previous key). This caused all second+ operations from the same account to fail with "Unable to get merkle proofs". - Add prev_commitment_pubkey parameter to send_coins - Mint handler derives it from minting account's num_pubkeys - 1 - Send handler accepts it from client as optional field - Add prev_pk derivation to test helper
send_coins returns commitment: None for user sends (server doesn't have the sender's private key). Previously this panicked with expect(). Now the inscription broadcast is skipped when no commitment is present, and the client can submit the signed commitment separately.
- receive_coin: enforce inclusion proof verification (was ignored) - mint/send handlers: replace .pop().unwrap() with safe match (prevents panic if coin_proofs is empty) - mint handler: log receive_coin errors instead of silently ignoring
Contributor
Post-merge: PRD data reset requiredThe MINTING_ADDRESS constant changed due to deterministic address derivation (commit d059b44). The old After merging and deploying to PRD, run: ssh dfxprd-remote 'docker stop zkcoins-server && \
docker run --rm -v zkcoins_server-data:/data alpine \
rm -f /data/accounts.bin /data/smt.bin /data/mmr.bin /data/mmr.bin.prev_root /data/latest_block.bin && \
docker start zkcoins-server'The server will recreate all state files on startup with the correct MINTING_ADDRESS. |
- Lint: cargo fmt --check, clippy on server/shared/program - Build: cargo build -p server - Tests: state + shared + program lib (skip slow SP1 prover tests) - Rust 1.81.0 pinned, cargo cache enabled
- Remove unused imports (Xpriv, lazy_static, XOnlyPublicKey, Network, hash, Rng) - Replace .get().is_none() with !contains_key() - Allow dead_code for ReceiveCoinRequest.coin_proof and serve_index()
15 tests covering: health, info, balance (valid/invalid/minting), mint, send (missing body/invalid JSON/no content-type), address, proof (not found), commit, unknown routes. Uses oneshot() pattern with shared test state.
- Publisher returns Err when no UTXOs available (was Ok(None)) - Mint handler returns 503 if inscription broadcast fails - Commit handler returns 503 if inscription broadcast fails - Client now gets explicit failure instead of false success
ProofStore now writes each proof as an individual file in /data/proofs/ instead of keeping them in-memory. On restart, scans the directory to restore the next_id counter. This ensures clients can still submit commitments after a server restart.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Reorder persistence: proof is written to disk FIRST, then accounts. If crash happens between the two writes, the proof exists (client can still commit) but the account balance is unchanged (no loss). Previously: accounts saved first → crash before proof write = balance decremented but proof lost = funds gone.
- UsernameStore with claim/resolve/reverse-lookup and file persistence
- POST /api/username: claim username with Schnorr signature auth
- GET /api/username/{username}: resolve username to address
- GET /.well-known/lnurlp/{username}: LUD-16 compatible payRequest
- GET /lnurl/pay/{username}: callback stub (Phase 2: Lightning)
- Balance response now includes username field
- Open CORS on LNURL endpoints for wallet compatibility
Resolve identifiers by checking custom usernames first, then
falling back to hex-prefix matching against known accounts.
Every account automatically gets {8-hex}@zkcoins.app.
- Flatten all routes onto single Router (no .nest() for LNURL) - Add State extractor to lnurl_callback_handler (required by Axum) - Use open CORS (Any origin) for all endpoints - Add 12 integration tests: username resolve, hex prefix lookup, LNURL payRequest, callback, claim validation, balance with username, concurrent reads
- 4 scanner tests: valid inscription, invalid data, signature verification after roundtrip, multi-chunk payload - 2 commit tests: non-existent proof_id returns 404, invalid signature returns 401
Replace mint-dependent test with simpler non-existent proof_id check that works without the full prover/broadcast stack.
- 6 tests for verify_send_signature: missing sig, missing timestamp, expired timestamp, invalid hex, wrong signature, valid signature - 1 test for UsernameStore file persistence roundtrip
- 3 claim tests: valid signature (e2e), wrong pubkey (401), expired (401) - 2 receive tests: duplicate coin rejected, balance updates after receive - 2 username tests: case-insensitive resolve, unknown address returns None - Total: 64 server tests passing
- account_server: log only first 2 bytes of recipient address instead of full Debug output (privacy for a shielded transaction system) - server: replace println of full send_result/minting_result (contains CoinProofs with addresses and amounts) with ok/err status only - ProofStore: extract proof_path() helper using Path::join instead of format!() string concatenation (addresses CodeQL path injection alert)
…aths - Remove println of minting_account_data.address in test functions (CodeQL flagged as cleartext logging of sensitive data) - ProofStore::proof_path: canonicalize base dir and verify resolved path stays within it (addresses path-injection alert, even though IDs are server-generated u64 values)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
Commits: 1 new commit(s)