refactor: test/bench: dedup Build{Crediting,Spending}Transaction() #17183
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
Member
|
re-run ci |
Contributor
Author
Thanks. Unfortunately it has failed again, with another reason this time though (Job
|
laanwj
reviewed
Oct 21, 2019
theStack
force-pushed
the
20191018-refactor-deduplicate_build_transaction_functions
branch
2 times, most recently
from
cd6e71b to
8fdd33b
Compare
Contributor
Author
|
Still not 100% sure about the name of the new file (currently named |
maflcko
reviewed
Oct 22, 2019
prototypes used in src/test/script_tests.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit); prototypes used in bench/verify_script.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit); The more generic versions from the script tests are moved into a new file pair transaction_utils.cpp/h and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for BuildCreditingTransaction(), passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in BuildSpendingTransaction()).
theStack
force-pushed
the
20191018-refactor-deduplicate_build_transaction_functions
branch
from
8fdd33b to
a0fc076
Compare
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Oct 23, 2019
…ding}Transaction() a0fc076 refactor: test/bench: dedup Build{Crediting,Spending}Transaction() (Sebastian Falbesoner) Pull request description: prototypes used in `src/test/script_tests.cpp`: - `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);` - `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);` prototypes used in `bench/verify_script.cpp`: - `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);` - `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);` The more generic versions from the script tests are moved into `setup_common.cpp` and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for `BuildCreditingTransaction()`, passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in `BuildSpendingTransaction()`). Top commit has no ACKs. Tree-SHA512: 8444f8a18f15070eeec1e5dfd255b55a851dfc2e6647c12b1995a6f7abd7196e830db2181d0e860bcd4cf4c815967584a3756dd450346bca70649dd1d4493e04
Member
|
Thanks. ACK |
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Apr 22, 2020
Summary: prototypes used in src/test/script_tests.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit); prototypes used in bench/verify_script.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit); The more generic versions from the script tests are moved into a new file pair transaction_utils.cpp/h and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for BuildCreditingTransaction(), passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in BuildSpendingTransaction()). This is a backport of Core [[bitcoin/bitcoin#17183 | PR17183]] The benchmark in question was deleted because segwit dependent, but it may be a good time to resurrect it. Test Plan: make check ninja check Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D5806
ftrader
pushed a commit
to bitcoin-cash-node/bitcoin-cash-node
that referenced
this pull request
Aug 17, 2020
Summary: prototypes used in src/test/script_tests.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit); prototypes used in bench/verify_script.cpp: - CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey); - CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit); The more generic versions from the script tests are moved into a new file pair transaction_utils.cpp/h and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for BuildCreditingTransaction(), passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in BuildSpendingTransaction()). This is a backport of Core [[bitcoin/bitcoin#17183 | PR17183]] The benchmark in question was deleted because segwit dependent, but it may be a good time to resurrect it. Test Plan: make check ninja check Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D5806
theStack
deleted the
20191018-refactor-deduplicate_build_transaction_functions
branch
11 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 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.
prototypes used in
src/test/script_tests.cpp:CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);prototypes used in
bench/verify_script.cpp:CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);The more generic versions from the script tests are moved into
setup_common.cppand the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely forBuildCreditingTransaction(), passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction inBuildSpendingTransaction()).