test: Fix intermittent feature_taproot issue #20292

MarcoFalke commented at 10:39 AM on November 3, 2020: member

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

Please refer to the commit messages for more information.

MarcoFalke force-pushed on Nov 3, 2020

MarcoFalke added this to the milestone 0.21.0 on Nov 3, 2020

DrahtBot commented at 11:06 AM on November 3, 2020: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19573 (Replace unused BIP 9 logic with draft BIP 8 by luke-jr)

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 added the label Tests on Nov 3, 2020

test: Remove unused unnamed parameter from block.serialize call

All blocks and transactions are serialized with witness, no need to set
this

fa762a3fd4

test: Fix deser issue in create_block

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

fa1dea19fc

test: Fix intermittent feature_taproot issue

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)

fac865b72d

Small improvements to the Taproot functional tests

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)

50eb0c2512

MarcoFalke force-pushed on Nov 3, 2020

ci: Bump timeout factor

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)

fab900802d

MarcoFalke force-pushed on Nov 3, 2020

in test/functional/feature_taproot.py:1455 in fab900802d

1455 | +                'vout': i['vout']
1456 | +            } for i in self.nodes[1].listunspent()],
1457 | +            outputs={addr: self.nodes[1].getbalance()},
1458 | +        )
1459 | +        rawtx = self.nodes[1].signrawtransactionwithwallet(rawtx)['hex']
1460 | +        # Transaction is too large to fit into the mempool, so put it into a block

mjdietzx commented at 8:12 PM on November 6, 2020:

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

MarcoFalke commented at 6:29 AM on November 7, 2020:

You can find where it is thrown by running git grep 'Transaction too large'

MarcoFalke commented at 6:29 AM on November 7, 2020:

The transaction is too large because it has too many inputs

laanwj commented at 2:46 PM on November 9, 2020: member

Code review ACK fab900802d74799ac1004265951be1b66e70cf76

laanwj merged this on Nov 9, 2020

laanwj closed this on Nov 9, 2020

MarcoFalke deleted the branch on Nov 9, 2020

sidhujag referenced this in commit 69e8e544e3 on Nov 9, 2020

DrahtBot locked this on Feb 15, 2022