[pull] master from bitcoin:master #1455
Merged
Conversation
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
Correct destructor implementation comment to no longer refer to shared pointers and also move it into the function body, in symmetry with Clone() right below. Leftover from #30866.
Makes a lot of fields in miniscript.h non-const in order to allow move-operations 2 commits later.
Also fixes adjacent comment typos.
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Functional parity is achieved through making Node move-able. Unfortunately ~Node() now needs to have the recursion linter disabled, as it is unable to figure out that recursion stops 1 level down. The former smart pointers must have been circumventing the linter somehow. NodeRef & MakeNodeRef() are deleted in the following commit (broken out to facilitate review).
(Also removes parameter to TestSatisfy() which existed unused from the start in 22c5b00).
Can be tested through emptying the function body of ~Node() or replacing Clone() implementation with naive version:
```C++
Node<Key> Clone() const
{
std::vector<Node> new_subs;
new_subs.reserve(subs.size());
for (const Node& child : subs) {
new_subs.push_back(child.Clone());
}
return Node{internal::NoDupCheck{}, m_script_ctx, fragment, std::move(new_subs), keys, data, k};
}
```
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
It will be used for errors related to CScriptNum (e.g. overflow or encoding errors). Currently, we simply return unknown error for these errors.
On Windows, the `winnt.h` header defines `DELETE` as a macro for a "Standard Access Right" bitmask (0x00010000L). This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before this enum definition, the preprocessor expands `DELETE` into a numeric literal, causing syntax errors. Rename the enumerator to `DELETE_FLAG` to remove this fragility and avoid the collision entirely.
…ETE_FLAG` 516be10 wallet: Rename `RecordType::DELETE` to `RecordType::DELETE_FLAG` (Hennadii Stepanov) Pull request description: On Windows, the `winnt.h` header defines `DELETE` as a macro for a "Standard Access Right" bitmask (0x00010000L). This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before the `RecordType` enum definition, the preprocessor expands `DELETE` into a numeric literal, causing syntax errors. Rename the enumerator to `DELETE_FLAG` to remove this fragility and avoid the collision entirely. Split from #34448. ACKs for top commit: maflcko: re-lgtm ACK 516be10 achow101: ACK 516be10 Tree-SHA512: eba054b395e18c07efb2901b28f542b042b62d85e1a798eeff35f8431530cb667fa791c47c4125cecdb689213b458ba396715495415e9b83bb322509a9376222
964c44c test(miniscript): Prove avoidance of stack overflow (Hodlinator) 198bbae refactor(miniscript): Destroy nodes one full subs-vector at a time (Hodlinator) 50cab85 refactor(miniscript): Remove NodeRef & MakeNodeRef() (Hodlinator) 15fb34d refactor(miniscript): Remove superfluous unique_ptr-indirection (Hodlinator) e55b23c refactor(miniscript): Remove Node::subs mutability (Hodlinator) c6f798b refactor(miniscript): Make fields non-const & private (Hodlinator) 22e4115 doc(miniscript): Remove mention of shared pointers (Hodlinator) Pull request description: Removes one level of unnecessary indirection, which was a change that originally [aided in finding one issue](#30866 (review)) in #30866. Simplifies the code one step further than 09a1875 belonging to aforementioned PR. Also adds test which verifies resistance to stack overflow when it comes to `~Node()` and `Node::Clone()`. No observed difference when running benchmarks: ExpandDescriptor/WalletIsMineDescriptors/WalletIsMineMigratedDescriptors/WalletLoadingDescriptors. Followup to #30866. ACKs for top commit: achow101: ACK 964c44c darosior: Code review ACK 964c44c l0rinc: ACK 964c44c Tree-SHA512: 32927e8f0f916fb70372ffd110f7ec7207d9e7a099c21c0a7482a12e96593b673c339719f4ab166ad7c086dc43767315fc1742c5b236a3facc45c4cfeb5872e9
6f7b432 test: remove UNKNOWN_ERROR from script_tests (Bruno Garcia) bd31a92 script: use SCRIPT_ERR_SCRIPTNUM for CScriptNum errors (Bruno Garcia) 0ca4dcd script: add SCRIPT_ERR_SCRIPTNUM error (Bruno Garcia) Pull request description: When evaluating a script, the current code is bad for analyzing some errors because it returns `SCRIPT_ERR_UNKNOWN_ERROR` for errors that are clearly known. `CScriptNum` has two well defined errors: number overflow and non-minimally encoded number. However, for both errors we return as unknown. This PR changes it by adding a new ScriptError that is used for any `CScriptNum` error. ACKs for top commit: achow101: ACK 6f7b432 w0xlt: ACK 6f7b432 darosior: ACK 6f7b432 Tree-SHA512: e656d9992251fbc95d33966fa18ce64bf714179d51ba6a7f429e5a55bc58e7fc08827e4ab71ace0dd385dac7e1feaea621b49503387793a30eae7a7e44aa6b0f
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )