Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 3, 2020

This fixes three bugs. Also, fix some unrelated code style issues.

Please refer to the commit messages for more information.

@maflcko maflcko added this to the 0.21.0 milestone Nov 3, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 3, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, 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.

@fanquake fanquake added the Tests label Nov 3, 2020
MarcoFalke and others added 4 commits November 3, 2020 12:23
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)
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)
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
Copy link
Contributor

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

Copy link
Member Author

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'

Copy link
Member Author

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

@laanwj
Copy link
Member

laanwj commented Nov 9, 2020

Code review ACK fab9008

@laanwj laanwj merged commit 4fd37d0 into bitcoin:master Nov 9, 2020
@maflcko maflcko deleted the 2011-testFixes branch November 9, 2020 15:00
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 9, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants