-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: Fix intermittent feature_taproot issue #20292
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
Conversation
a7c8e8f
to
5a51fa8
Compare
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. |
All blocks and transactions are serialized with witness, no need to set this
Without the fix a hex-string can not be parsed: File "./test/functional/test_framework/blocktools.py", line 82, in create_block txo.deserialize(io.BytesIO(tx)) TypeError: a bytes-like object is required, not 'str' Also, remove io import and repace it with our FromHex() helper
The transaction is too large to fit into the mempool, so put it into a block. https://travis-ci.org/github/bitcoin/bitcoin/jobs/740987240#L7217 test 2020-11-03T01:31:08.645000Z TestFramework (ERROR): JSONRPC error Traceback (most recent call last): File "./test/functional/test_framework/test_framework.py", line 126, in main self.run_test() File "./test/functional/feature_taproot.py", line 1448, in run_test self.nodes[1].sendtoaddress(address=addr, amount=int(self.nodes[1].getbalance() * 70000000) / 100000000) File "./test/functional/test_framework/coverage.py", line 47, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) File "./test/functional/test_framework/authproxy.py", line 146, in __call__ raise JSONRPCException(response['error'], status) test_framework.authproxy.JSONRPCException: Transaction too large (-6)
The "whitelist" and "connect_nodes" is not needed in feature_taproot.py, so remove it. The changes to key.py are required when running the unit tests from the test folder. Failure on current master: [test]$ python -m unittest functional/test_framework/key.py .E ====================================================================== ERROR: test_schnorr_testvectors (functional.test_framework.key.TestFrameworkKey) Implement the BIP340 test vectors (read from bip340_test_vectors.csv). ---------------------------------------------------------------------- Traceback (most recent call last): File "test/functional/test_framework/key.py", line 526, in test_schnorr_testvectors with open(os.path.join(sys.path[0], 'test_framework', 'bip340_test_vectors.csv'), newline='', encoding='utf8') as csvfile: FileNotFoundError: [Errno 2] No such file or directory: 'test/test_framework/bip340_test_vectors.csv' ---------------------------------------------------------------------- Ran 2 tests in 0.775s FAILED (errors=1)
5a51fa8
to
50eb0c2
Compare
fab94d9
to
fafe620
Compare
This avoids timeouts when signing a large raw transaction https://cirrus-ci.com/task/5009228131729408?command=ci#L4981 test_framework.authproxy.JSONRPCException: 'signrawtransactionwithwallet' RPC took longer than 120.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
fafe620
to
fab9008
Compare
outputs={addr: self.nodes[1].getbalance()}, | ||
) | ||
rawtx = self.nodes[1].signrawtransactionwithwallet(rawtx)['hex'] | ||
# Transaction is too large to fit into the mempool, so put it into a block |
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.
Where does the error get thrown if you were to send the transaction normally? And why is it too large atm? Everything in the PR looks good, but I thought I should understand this before ack, and haven't been able to figure out
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.
You can find where it is thrown by running git grep 'Transaction too large'
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.
The transaction is too large because it has too many inputs
Code review ACK fab9008 |
This fixes three bugs. Also, fix some unrelated code style issues.
Please refer to the commit messages for more information.