Open
Conversation
Signed-off-by: b-l-u-e <winnie.gitau282@gmail.com>
Signed-off-by: b-l-u-e <winnie.gitau282@gmail.com>
Signed-off-by: b-l-u-e <winnie.gitau282@gmail.com>
This is required in the next commit.
This refactor is a follow-up to commit eeeeb2a and does not change any behavior. However, it is nice to know that no global mocktime leaks from the fuzz init step to the first fuzz input, or from one fuzz input execution to the next. With the clock context, the global is re-set at the end of the context.
Some tests spell a two-value membership check as `assert x == a or x == b`. Rewrite those sites as `assert x in (a, b)`. This keeps the check the same and removes `==` forms that the later cleanup should not touch.
Some tests combine multiple equality checks in one `assert`. Split those checks into separate assertions so failures point at the exact mismatch. This also removes mixed `==` expressions that the later cleanup should not touch.
The later scripted diff only handles plain `assert x == y` lines. Some remaining tests still use equality inside comprehensions, parenthesized asserts, and other shapes that the line-based rewrite would misread. Rewrite those sites by hand first so the later mechanical conversion stays safe. The commit also simplifies the dead `len(["errors"]) == 0` branch in `blocktools.py`, which can never be true.
Some remaining `assert x == y` lines include inline comments, list literals, or descriptor strings with `#`. Convert those sites by hand so the later mechanical rewrite can stay simple.
A few files still need to stay out of the later scripted diff because they contain more complicated assert shapes. Convert the straightforward equality checks in those files by hand first. Use a local import in `authproxy.py` so the change does not create an import cycle.
Replace the last remaining truthy asserts (and a remaining `is False`) in `wallet_miniscript.py` and `rpc_psbt.py` with `assert_equal`.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
The later scripted diff only rewrites call sites. Add `assert_equal` imports in the remaining files that still need them so the mechanical replacement can apply cleanly.
Some Python functional tests still use plain `assert x == y`. The earlier commits convert the ambiguous assert patterns by hand, so this commit can rewrite the remaining safe cases mechanically. The verify script excludes `wallet_bumpfee.py`, `test_framework/netutil.py`, and `test_framework/authproxy.py`, which still contain assert forms that the plain line-based substitution would misidentify. -BEGIN VERIFY SCRIPT- perl -pi -e 's/^(\s*)assert (.+?) == ([^,#]+?)$/\1assert_equal(\2, \3)/' $(git ls-files -- 'test/functional' \ ':(exclude)test/functional/wallet_bumpfee.py' ':(exclude)test/functional/test_framework/netutil.py' ':(exclude)test/functional/test_framework/authproxy.py') -END VERIFY SCRIPT-
Reduces risk for accidental use of costly TestChain100Setup in later added test cases.
The most frequent change to CoinSelectionParams so far seems to be amendments to the feerate, and the feerate propagates to other members of the CoinSelectionParams, so we add it as an input parameter. This also adds the CoinSelectionParams to the BnBFail tests, which previously were always run with the default CoinSelectionParams.
The default minimum feerate was lowered from 1000 s/kvB to 100 s/kvB. This adjusts the set of feerates used in the tests to accommodate that new feerate and to cover other potential special cases: - zero: 0 s/kvB - minimum non-zero s/kvB: 1 s/kvB - just below the new default minimum feerate: 99 s/kvB - new default minimum feerate: 100 s/kvB - old default minimum feerate: 1000 s/kvB - a few non-round realistic feerates around default minimum feerate, dust feerate, and default LTFRE: 315 s/kvB, 2345 s/kvB, and 10'292 s/kvB - a high feerate that has been exceeded occassionally: 59'764 s/kvB - a huge feerate that is extremely uncommon: 1'500'000 s/kvB
This refactor is part of the effort to move the coin selection tests to a framework that can use non-zero, realistic feerates. The insufficient funds failure case is extended with a few additional similar variants of the failure.
Previously, SRD had only failure and edge-case coverage, so we add a few simple coin selection targets that should quickly succeed, including one that will create a minimal change output.
Also: - Add weight check to Success cases. Per the introduction of TRUC transactions, it is more likely that we will attempt to build transactions of limited weight (e.g., TRUC child transactions may not exceed 4000 WU). When SRD exceeds the input weight limit, it evicts the OutputGroup with the lowest effective value before selecting additional UTXOs: we test that SRD will find a solution that depends on the eviction working correctly, and that it fails as expected when no solution is possible.
since ed764ea, all descendants in m_block_index are required to be marked BLOCK_FAILED_VALID when an invalid block is encountered, as enforced by a CheckBlockIndex assert. remove the now-redundant marking in FindMostWorkChain's inner loop, so it is only responsible for cleaning up setBlockIndexCandidates, not for modifying block validity state
InvalidateBlock propagates to all descendants in m_block_index, but ReconsiderBlock only clears blocks on the same chain as the reconsidered block (its direct ancestors and descendants). Blocks on sibling forks are not cleared. This asymmetry can lead to edge cases like in issue bitcoin#32173 (fixed in 37bc207). Add a unit test for this scenario to safeguard against future regressions.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Standardize coins view naming with a mechanical rename pass.
This keeps subsequent commits focused on the interface and behavioral changes.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
-BEGIN VERIFY SCRIPT-
git grep -qE '\bin_base\b|\bin_view\b|\bin_block_hash\b|\bblock_hash\b' -- src/coins.cpp src/coins.h src/txdb.cpp src/txdb.h src/test/coins_tests.cpp && { echo "Error: target names already exist in scoped files"; exit 1; }
perl -pi -e '
s/\bbaseIn\b/in_base/g;
s/\bhashBlockIn\b/in_block_hash/g;
s/\bhashBlock\b/block_hash/g;
s/\bviewIn\b/in_view/g;
' src/coins.cpp src/coins.h src/txdb.cpp src/txdb.h src/test/coins_tests.cpp
-END VERIFY SCRIPT-
Let's get these out of the way to simplify riskier followup commits
`TestCoinsView` switches the `CCoinsViewCache` backend during fuzzing and then queries the backend for cross-checks.
Pass the backend as a `CCoinsView*` (to make it relocatable) and retarget it when toggling between the original backend and a local empty `CCoinsView` so assertions always refer to the active backend. This will be switched to a singleton in the next commit.
Note that the previous slice-assignment (`backend_coins_view = CCoinsView{}`) was a silent noop for the db target, it only copied base-class members (none) without changing the vtable, so the backend was never actually switched.
The pointer approach makes the switch real for both targets, which revealed that when restoring the original backend after the empty one, the cache must be reset first to avoid carrying FRESH flags that were valid relative to the empty backend but invalid relative to the original (which may already contain those coins).
Co-authored-by: Anthony Towns <aj@erisian.com.au>
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: marcofleon <marleo23@proton.me>
Introduce `CoinsViewEmpty` as an explicit no-op `CCoinsView` implementation, and define its singleton accessor out of line in `coins.cpp` to avoid `-Wunique-object-duplication` in shared-library builds.` Use it at call sites that intentionally want a no-op backend instead of constructing anonymous placeholder views. `CCoinsViewTest` and `CoinsViewBottom` now inherit defaults from `CoinsViewEmpty` (e.g. the unused `EstimateSize()`, which now returns 0). Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
`CCoinsView` provided default no-op implementations, which allowed constructing a bare view and silently getting dummy behavior. Make all interface methods pure virtual and remove the legacy default definitions from `coins.cpp` so callers must choose an explicit implementation. Move the virtual destructor to the beginning to avoid mixing it between the methods. No-op backing behavior remains available via `CoinsViewEmpty`.
…r-notes ea893cf doc: fix typo 'parlor' to 'parlance' in developer-notes (ArvinFarrelP) Pull request description: Fix a small typo in doc/developer-notes.md. Replace "parlor" with "parlance" to use the correct term in the context of C++ terminology. ACKs for top commit: maflcko: lgtm ACK ea893cf l0rinc: ACK ea893cf Tree-SHA512: 5437e0eb1b40aa1eafe8a0482350ed6259ec3492f64e7ba8d046358a3dd9cf95bfc38825b781350fd321c453cc23dddb6a14fe7f5087e56c77a17e3d3e233de4
dfd54c9 Squashed 'src/secp256k1/' changes from 57315a6985..7262adb4b4 (fanquake) Pull request description: bitcoin-core/secp256k1#1760 was merged upstream, which improves test parallelism, and reduces overall runtime. Our longest running tests are secp256k1 tests (`secp256k1_tests`), so we might want to pull this down, to speedup all of our ctest invocations. The new upstream code increases output verbosity, i.e the test list is now ~330, and we have output like: ```bash Label Time Summary: secp256k1_exhaustive = 19.52 sec*proc (1 test) secp256k1_noverify_tests = 45.65 sec*proc (91 tests) secp256k1_tests = 90.55 sec*proc (91 tests) ``` maybe we can/should mute that somewhat? ACKs for top commit: hebasto: ACK b7f9178. Tree-SHA512: 098a8b54d3a489e6ad7866f4e5152a5bc6a0e1da69b2a84d8795423e64faa42772cbcb194feac228a547869d2b3be2198aaf96a5e1bb7d45a2e385fd5e78bafd
…o `assert_equal` 3fd68a9 scripted-diff: replace remaining Python test equality asserts (Lőrinc) 301b1d7 test: add missing `assert_equal` imports (Lőrinc) 06a4176 test: convert truthy asserts in `wallet_miniscript` and `rpc_psbt` (Lőrinc) d9a3cf2 test: convert simple equality asserts in excluded files (Lőrinc) 23c06d4 test: convert equality asserts with comments or special chars (Lőrinc) dcd90fb test: prep manual equality assert conversions (Lőrinc) 4f4516e test: split equality asserts joined by `and` (Lőrinc) 76a5570 test: use `in` for two-value equality asserts (Lőrinc) Pull request description: ### Problem Plain `assert x == y` is a poor fit for this test framework, `assert_equal()` gives more useful failure output and keeps equality checks consistent with the rest of the functional tests. ### Design A simple scripted diff cannot safely rewrite all of them because many files use `==` inside larger expressions, such as chained conditions, comprehensions, and other compound assertions. That makes a one-shot mechanical conversion either incorrect or harder to review. ### Fix This series first rewrites the non-mechanical cases into standalone assertions so patterns are easier to identify, then applies a scripted diff to the remaining cases that are safe to convert mechanically. Partially fixes bitcoin#23119 by adjusting the `==` case only (which is similar to the `BOOST` alternatives so it's expected to be uncontroversial). ACKs for top commit: maflcko: review ACK 3fd68a9 🚆 rkrux: lgtm ACK 3fd68a9 theStack: ACK 3fd68a9 Tree-SHA512: 6950ffa044db72d6235305f4c0247254e7e8f57ee1c8300e553953963914a6360ca71569fe315ecb333cf0e62f78b3a24603717f64229783761f8c1b5958fc12
…erface 8783cc8 refactor: inline `CCoinsViewBacked` implementation (Lőrinc) 86296f2 coins: make `CCoinsView` methods pure virtual (Lőrinc) b637566 coins: add explicit `CoinsViewEmpty` noop backend (Lőrinc) 90c635c fuzz: keep backend assertions aligned to active backend (Lőrinc) a9f92e3 refactor: normalize CCoinsView whitespace and signatures (Lőrinc) 38a99f3 scripted-diff: normalize `CCoinsView` naming (Lőrinc) 06172ef refactor: rename `hashBlock` to `m_block_hash` to avoid shadowing (Lőrinc) Pull request description: ### Problem `CCoinsView` is the coins view interface, but historically it also provided built-in no-op behavior: * It provided default no-op implementations (returning `std::nullopt`, `uint256()`, `false`, or `nullptr`) instead of being pure virtual. * Callers could instantiate a bare `CCoinsView` and get silent no-op behavior. * Mixing the interface definition with a built-in dummy implementation blurred the abstraction boundary. ### Context This is part of the ongoing coins caching cleanup in bitcoin#34280. ### Fix This PR separates the interface from no-op behavior and makes `CCoinsView` pure virtual in incremental steps: * Add `CoinsViewEmpty` as an explicit no-op coins view for tests, benchmarks, and temporary backends. * Replace direct bare-`CCoinsView` test and dummy instantiations with `CoinsViewEmpty`. * Make all `CCoinsView` methods pure virtual (`PeekCoin`, `GetCoin`, `HaveCoin`, `GetBestBlock`, `GetHeadBlocks`, `BatchWrite`, `Cursor`, `EstimateSize`). * Remove the legacy default implementations from `coins.cpp`. * Update fuzz and dummy backends to use `CoinsViewEmpty` explicitly. ACKs for top commit: w0xlt: reACK 8783cc8 ryanofsky: Code review ACK 8783cc8. Just updated comments and variable name since last review. The fuzz test code is much clearer now IMO andrewtoth-exo: ACK 8783cc8 ajtowns: ACK 8783cc8 Tree-SHA512: cfc831578aa309788c1b5dafbfecca3de388cc4215534c3f3df24f90d7770ed37b1fd7aa134df91d611d0a1ca75929accb98d5ed7df7b52851c259e04f08e4a3
pythcoiner
force-pushed
the
wip-encrypted-backup
branch
3 times, most recently
from
2607ce6 to
ea61634
Compare
Replace magic 0x80000000 literals with named constants for BIP32 hardened/unhardened child derivation.
Extract ParseKeyPathElement() as a shared helper in util/bip32 that parses a single BIP32 key path element (e.g. "0", "0'", "0h"). Both ParseHDKeypath() and the descriptor parser's ParseKeyPathNum() now delegate to it, eliminating duplicated parsing logic.
Add helper methods to AEADChaCha20Poly1305 for converting between
the internal Nonce96 type ({uint32_t, uint64_t}) and a 12-byte
array representation (big-endian).
RFC8439 defines the nonce as 96 opaque bits, but our implementation
splits it. These helpers make it convenient to work with byte-based
nonce representations.
BIP-380 specifies that descriptors can use either ' or h as the hardened indicator. ParseHDKeypath only supported the former. This prepares for using ParseHDKeypath with paths extracted from descriptors which typically use 'h' for shell-escaping convenience.
Introduces WalletDescriptorInfo struct and DescriptorInfoToUniValue() helper to avoid code duplication when serializing descriptor metadata to UniValue. Refactors listdescriptors RPC to use the new helper.
Add functions for normalizing public keys to x-only format as specified in BIP-xxxx (Bitcoin Encrypted Backup). These primitives form the foundation for the encryption scheme. Functions added: - NormalizeToXOnly(): Convert CPubKey or CExtPubKey to 32-byte x-only format - IsNUMSPoint(): Check if a key is the BIP341 unspendable NUMS point - ExtractKeysFromDescriptor(): Extract and normalize all keys from a descriptor Includes test vectors from the BIP specification.
Add functions to compute the decryption secret and individual secrets as specified in BIP-xxxx. The decryption secret is derived from sorted public keys, and individual secrets allow any keyholder to decrypt. Functions added: - ComputeDecryptionSecret(): Hash sorted keys to derive decryption secret - ComputeIndividualSecret(): Hash a single key to derive its individual secret - ComputeAllIndividualSecrets(): Compute XOR'd secrets for all keys Includes test vectors from the BIP specification.
Add functions for encoding/decoding derivation paths as specified in BIP-xxxx: - ParseDerivationPath: parse m/44'/0'/0' style strings - EncodeDerivationPaths: encode to binary format (count + path lengths + BE child indices) - DecodeDerivationPaths: decode from binary format Includes test vectors from the BIP specification.
Add functions for encoding/decoding individual secrets as specified in BIP-xxxx:
- EncodeIndividualSecrets: encode sorted secrets to binary format
- DecodeIndividualSecrets: decode from binary format
Individual secrets allow any keyholder to derive the decryption secret using:
decryption_secret = ci XOR sha256("BIP_XXXX_INDIVIDUAL_SECRET" || pi)
Includes test vectors from the BIP specification.
Add functions for encoding/decoding content type metadata as specified in BIP-xxxx: - EncodeContent: encode BIP_NUMBER or VENDOR_SPECIFIC content types - DecodeContent: decode content type from binary format Content types define what kind of data is in the encrypted payload: - BIP_NUMBER: references a BIP specification (e.g., 380 for descriptors) - VENDOR_SPECIFIC: application-defined content with length prefix Includes test vectors from the BIP specification.
Add the AEAD primitives used to encrypt and decrypt the backup payload: - EncryptChaCha20Poly1305: encrypt with ChaCha20-Poly1305 AEAD - DecryptChaCha20Poly1305: decrypt and verify the authentication tag These wrap AEADChaCha20Poly1305 from src/crypto/, exposing a uint8_t interface keyed on the BIP-xxxx 32-byte secret and 12-byte nonce.
Add the EncryptedBackup struct and the binary/base64 wire encoding: - EncodeEncryptedBackup: encode to binary format - EncodeEncryptedBackupBase64: encode to base64 string - DecodeEncryptedBackup: decode from binary format - DecodeEncryptedBackupBase64: decode from base64 string The format uses 6-byte magic "BIPXXX", version byte, derivation paths, individual secrets (ci values for key recovery), encryption algorithm identifier, nonce, and CompactSize-prefixed ciphertext.
Add the high-level API tying the primitives together: - CreateEncryptedBackup: create a backup from a descriptor and plaintext - DecryptBackupWithKey: attempt decryption using a single public key - DecryptBackupWithDescriptor: try all keys from a descriptor Decryption reconstructs the per-key share via si = sha256(tag || pi) and recovers the decryption secret as s = ci XOR si for each candidate ci in the backup, then attempts AEAD decryption.
Stateless RPC that takes a descriptor string and returns a base64- encoded BIP-XXXX encrypted backup. The encryption key is derived from the public keys in the descriptor. No wallet is needed.
Stateless RPC that takes a base64-encoded BIP-XXXX encrypted backup and an extended public key (xpub/tpub), then returns the decrypted descriptor string. No wallet is needed.
Decrypt a BIP-XXXX encrypted backup and import the descriptor into the loaded wallet. Requires -rpcwallet. Takes a base64 backup and an extended public key (xpub/tpub).
Stateless RPC that takes a base64-encoded BIP-XXXX encrypted backup and returns its unencrypted metadata: version, number of recipients, encryption algorithm, and derivation paths. No wallet is needed.
Stateless command that takes a descriptor string via -descriptor and encrypts it using the BIP-XXXX encrypted backup format. The plaintext is the raw descriptor string bytes. Output is base64 to stdout by default, or raw binary to -backupfile. No wallet file is loaded — the encryption key is derived from the public keys in the descriptor itself.
Stateless command that takes an encrypted backup (base64 on stdin or raw binary via -backupfile) and a -pubkey (xpub/tpub), then outputs the decrypted descriptor string to stdout. No wallet file is loaded.
Decrypt an encrypted backup and import the descriptor into an existing wallet file. Requires -wallet, -pubkey, and backup data (base64 on stdin or raw binary via -backupfile). Also extracts ReadBackupFromArgsOrStdin() helper to avoid duplicating the backup-reading logic across decrypt/import/inspect commands.
Display unencrypted metadata from a BIP-xxxx encrypted backup: - Format version - Number of recipients - Encryption algorithm - Derivation paths (if present)
pythcoiner
force-pushed
the
wip-encrypted-backup
branch
from
ea61634 to
dfadaa3
Compare
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
12 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.
No description provided.