rpc_signtransactionwithkey.py
#30667
send_to_address functional test in rpc_signrawtransactionwithkey.py. The code is more terse with a MiniWallet object, its ADDRESS_OP_TRUE mode and send_to method on line 54, thus removing the “low-level” details of the transaction. Suggestions will be addressed. Thank you for reviewing!
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept NACK | ismaelsadeeq |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/28879496602
Make sure to run all tests locally, according to the documentation.
The failure may happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
Thank you for attempting to address this issue.
The changes in this PR are incorrect and do not address the issue. Therefore, I am inclined to approach NACK. If you have any questions about this PR, please ask them in the issue and review the guidelines at CONTRIBUTING.md before opening a new PR.
Hint to anyone wanting to work on this.
check MiniWallet::send_to method.
https://github.com/bitcoin/bitcoin/blob/ee367170cb2acf82b6ff8e0ccdbc1cce09730662/test/functional/test_framework/wallet.py#L273-L294
And convert the address in SignRawTransactionWithKeyTest::send_to_address to a scriptPubKey before passing it to MiniWallet::send_to method.
57- output = {addr: amount}
58- self.blk_idx += 1
59- rawtx = self.nodes[0].createrawtransaction([input], output)
60- txid = self.nodes[0].sendrawtransaction(self.nodes[0].signrawtransactionwithkey(rawtx, [self.nodes[0].get_deterministic_priv_key().key])["hex"], 0)
61+ script_pub_key = address_to_scriptpubkey(addr).hex()
62+ wallet = MiniWallet(self.node[0], mode=MiniWalletMode.ADDRESS_OP_TRUE)
signrawtransactionwithkey works. Replacing the call to signrawtransactionwithkey with a different signing function seems wrong.
signrawtransactionwithkey is tested in other parts of this functional test.
I’m going to close this pull request for now. If you’d like to work on the issue, here are some hints before you open a new pull request.
node isn’t an attribute; you forgot an s. MiniWallet does not have a sendrawtransaction function; you perhaps confused that with send_to_address. The new wallet also doesn’t have any utxos to spend; it should probably be instantiated at the start of the test and be reused.