-
Notifications
You must be signed in to change notification settings - Fork 1.2k
DIP3 and DIP4 unit tests #2264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DIP3 and DIP4 unit tests #2264
Conversation
PastaPastaPasta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See in-line, minor formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backet should be on prior line
d55e37c to
2102217
Compare
Allows testing of features which depend on BIP9 deployments
We need to update the CbTx's merkle root for the masternode list here as tests might add transactions into a block which cause the list to change.
2102217 to
bc7924d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ACK
, bitcoin#25616, bitcoin#26005, bitcoin#24855, bitcoin#18554, bitcoin#17204, bitcoin#20562, bitcoin#21166, bitcoin#25044, bitcoin#25364, bitcoin#25525, bitcoin#25512, bitcoin#24678, bitcoin#26747, partial bitcoin#22154, bitcoin#24584 (wallet backports: part 8) c0f9225 merge bitcoin#26747: fix confusing error / GUI crash on cross-chain legacy wallet restore (Kittywhiskers Van Gogh) d63ca8a merge bitcoin#24678: Prevent wallet unload on GetWalletForJSONRPCRequest (Kittywhiskers Van Gogh) 2a880d8 merge bitcoin#25512: remove wallet dependency and refactor rpc_signrawtransaction.py (Kittywhiskers Van Gogh) f405790 merge bitcoin#25525: remove wallet dependency from mempool_updatefromblock.py (Kittywhiskers Van Gogh) c24c9ea merge bitcoin#25364: remove wallet dependency from feature_nulldummy.py (Kittywhiskers Van Gogh) a118fe1 test: reduce `num_nodes` in `feature_nulldummy.py` to 1 (Kittywhiskers Van Gogh) c6eabc1 merge bitcoin#25044: Use MiniWallet in rpc_rawtransaction.py (Kittywhiskers Van Gogh) 4e0725e merge bitcoin#21166: Introduce DeferredSignatureChecker and have SignatureExtractorClass subclass it (Kittywhiskers Van Gogh) f883122 merge bitcoin#20562: Test that a fully signed tx given to signrawtx is unchanged (Kittywhiskers Van Gogh) c349dad merge bitcoin#17204: Do not turn OP_1NEGATE in scriptSig into 0x0181 in signing code (Kittywhiskers Van Gogh) c5e8bd6 merge bitcoin#18554: ensure wallet files are not reused across chains (Kittywhiskers Van Gogh) 0fca914 merge bitcoin#24855: Fix `setwalletflag` disabling of flags (Kittywhiskers Van Gogh) f15a1b9 merge bitcoin#26005: Fix error handling (copy_file failure in RestoreWallet, and in general via interfaces) (Kittywhiskers Van Gogh) 240765e merge bitcoin#25616: Return `util::Result` from WalletLoader methods (Kittywhiskers Van Gogh) bd897fa merge bitcoin#25656: return util::Result from `GetReservedDestination` methods (Kittywhiskers Van Gogh) 56accfe merge bitcoin#25721: Replace BResult with util::Result (Kittywhiskers Van Gogh) 03939f2 partial bitcoin#24584: avoid mixing different `OutputTypes` during coin selection (Kittywhiskers Van Gogh) 50ca8cf refactor: remove `CKey` overload for `Create{,AndProcess}Block()` (Kittywhiskers Van Gogh) 9e23b11 merge bitcoin#25218: introduce generic 'Result' class and connect it to CreateTransaction and GetNewDestination (Kittywhiskers Van Gogh) ec764fd partial bitcoin#22154: Add OutputType::BECH32M and related wallet support for fetching bech32m addresses (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * The `CKey` overload for `Create{,AndProcess}Block()` was introduced as a convenience function in [dash#2264](#2264). To allow for the easier backport of [bitcoin#24584](bitcoin#24584) (by resolving an ambiguous overload error) and because `GetScriptForRawPubKey` makes the same construction, it was opted to realign with upstream and drop the overload. * Even though we don't actively support any address types that utilize Bech32m, [bitcoin#25656](bitcoin#25656) assumes that `GetReservedDestination` modifies a `bilingual_str` for its error case in order to encapsulate it in a `util::Result`. In order to complete that backport, [bitcoin#22154](bitcoin#22154) has been partially backported. * `CKeyHolder` has always assumed that `GetReservedDestination` will succeed without validation of this assumption ([source](https://github.com/dashpay/dash/blob/develop/src/coinjoin/util.cpp#L29-L33)). As [bitcoin#25656](bitcoin#25656) forces us to unwrap the returned value, this assumption has been documented with an `assert` ([source](https://github.com/dashpay/dash/blob/bd897fa7097d3d68d19b8a9af14b23f4007645ca/src/coinjoin/util.cpp#L32-L34)) * When backporting [bitcoin#21166](bitcoin#21166), we don't have the luxury of a `scriptWitness` to include the redemption script, so `rpc_signrawtransaction.py` must import the script in order to spend it. This creates an additional complication as descriptor wallets do not permit wallets to mix descriptors with and without private keys ([source](https://github.com/dashpay/dash/blob/cc9da5d9e28bf9a7b411dc390523ac6d2dd09de6/src/wallet/rpc/backup.cpp#L1758)) and as P2SH descriptors don't involve a private key, those specific subtests need to be skipped for descriptor wallets. * `OP_TRUE` has been appended to the script, this mirrors the `OP_TRUE` in the `scriptWitness` ([source](https://github.com/bitcoin/bitcoin/blob/a97a9298cea085858e1a65a5e9b20d7a9e0f7303/test/functional/rpc_signrawtransaction.py#L270)) * The subtest `test_signing_with_missing_prevtx_info()` introduced in [bitcoin#25044](bitcoin#25044) requires an extra `walletpassphrase` as the preceding tests that unlock the wallet are not called for descriptor wallets, making the call necessary. It is redundant but harmless for legacy wallets. * With the backport of [bitcoin#19937](bitcoin#19937) in [dash#6726](#6726) (as ae7e4cb), we can remove a workaround in `feature_nulldummy.py` where we used two nodes to satisfy `getblocktemplate`'s requirement of a connected node as that requirement was lifted on test chains. ## Breaking Changes None expected. ## How Has This Been Tested? A full CoinJoin session run on c0f9225  ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK c0f9225 UdjinM6: utACK c0f9225 Tree-SHA512: 2217281ea75afe3eb6061e1acbd42afb66ff2ab28c15a3ad03bdb528ef4e40ea30bf1adad8d0ba93310a3d561d6c3f2ad95780b0b972af238523c6f2950a39c7
No description provided.