Integrate PR #57 audit hardening onto master (Dilithium/P2MR/constants) + close audit items#61
Open
BarneyChambers wants to merge 1 commit into
Open
Integrate PR #57 audit hardening onto master (Dilithium/P2MR/constants) + close audit items#61BarneyChambers wants to merge 1 commit into
BarneyChambers wants to merge 1 commit into
Conversation
…g-gating + close audit items Brings the PR #57 wallet/Dilithium/P2MR/constants hardening up to date with master (PR #56 + #59 "activate Taproot / close second-pass audit gaps") and reconciles the divergences between the two lines of work. Reconciliation: - Encrypted Dilithium key IV standardized on DeriveDilithiumKeyIV() with a legacy raw-keyid decrypt fallback (crypter.cpp, scriptpubkeyman.cpp). - Dilithium script verification gated via the buried DEPLOYMENT_DILITHIUM (master/#59 design); SCRIPT_VERIFY_DILITHIUM removed from STANDARD_SCRIPT_VERIFY_FLAGS and applied explicitly in ProduceSignature()'s solution check (so Dilithium satisfactions still validate). - chainparams: mainnet defaultAssumeValid points at the re-mined genesis. - GetTxSigOpCost expects SCRIPT_ERR_EQUALVERIFY (matches the merged VerifyWitnessProgram), not the superseded WITNESS_PUBKEYTYPE. Audit fixes: - BTQ-AUDIT-021: CDilithiumKey::MakeNewKey() failure is now checked by its callers (DilithiumWalletManager, CUnifiedKey) instead of silently ignored. - BTQ-AUDIT-019: added P2SH-wrapped Dilithium sigop-count test coverage. - Strengthened wallet_dilithium_send.py to prove Dilithium sends are consensus-valid (mined confirmation + input actually spent + sendmany). All Critical/High audit findings are fixed; build is clean and the unit suites touched here pass. Pre-existing inherited-test failures (bloom_tests hardcoded WIF, bip324_tests vectors) are tracked separately in #60.
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
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.
Summary
Brings the PR #57 wallet/Dilithium/P2MR/constants audit hardening up to date with current
master(PR #56 + #59 "activate Taproot / close second-pass audit gaps"), reconciles the divergences between the two, and closes the remaining concrete audit items. Diff is the PR #57 work + reconciliation + audit fixes only (master's own #59 content is not part of this diff).What's in here
Merge reconciliation (where #57 and #59 overlapped)
DeriveDilithiumKeyIV()with a legacy raw-keyid decrypt fallback (crypter.cpp,scriptpubkeyman.cpp).DEPLOYMENT_DILITHIUM(master/fix(consensus): activate Taproot and close second-pass audit gaps #59 design, audit-017);SCRIPT_VERIFY_DILITHIUMremoved fromSTANDARD_SCRIPT_VERIFY_FLAGSand applied explicitly inProduceSignature()'s solution check so Dilithium satisfactions still validate.chainparams: mainnetdefaultAssumeValidpoints at the re-mined genesis (not the stale pre-remine hash).GetTxSigOpCostexpectsSCRIPT_ERR_EQUALVERIFY, matching the mergedVerifyWitnessProgram(the supersededWITNESS_PUBKEYTYPEexpectation was removed).Audit fixes
CDilithiumKey::MakeNewKey()RNG-failure return is now checked by its callers (DilithiumWalletManager,CUnifiedKey) instead of being silently dropped.wallet_dilithium_send.py(issue sendtoaddress returns-1: map::aton v0.3.2-testnet despite successful broadcast #41 regression) to prove Dilithium sends are consensus-valid: mined confirmation, the Dilithium UTXO is the input actually spent, plus asendmanycase.Audit status
All Critical/High findings from the internal audit are fixed. Remaining items are Medium/Low and non-blocking (signet placeholder challenge #4 [signet-only], doc/comment drift #3/#5/#24, accepted-with-rationale #25, pre-launch chainwork/assumevalid #27).
Test plan
btqd+src/test/test_btqbuild cleandilithium_basic_tests,dilithium_wallet_tests,scriptpubkeyman_tests,sigopcount_tests,feebumper_tests,p2mr_tests)wallet_dilithium_send.pyregression passes (Dilithium spend confirms in a block)test_btqrun is currently blocked by pre-existing inherited-test failures (bloom_testshardcoded WIF,bip324_testsvectors) tracked in test: inherited Bitcoin test vectors fail under BTQ params (bloom_tests, bip324_tests) and abort the unit suite #60 — these are identical tomasterand not introduced here.Notes