Skip to content
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

Remove manual byte editing in wallet_tx_clone func test #15397

Merged
merged 1 commit into from Feb 18, 2019

Conversation

Projects
None yet
5 participants
@instagibbs
Copy link
Member

commented Feb 13, 2019

Adapted from @stevenroose

@stevenroose

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

@MarcoFalke MarcoFalke added this to the 0.19.0 milestone Feb 13, 2019


# Use a different signature hash type to sign. This creates an equivalent but malleated clone.
# Don't send the clone anywhere yet
tx1_clone = self.nodes[0].signrawtransactionwithwallet(clone_raw, None, "ALL|ANYONECANPAY")
tx1_clone = self.nodes[0].signrawtransactionwithwallet(clone_tx.serialize().hex(), None, "ALL|ANYONECANPAY")

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 13, 2019

Member

Assigned 0.19.0, because this is python3.5 only

This comment has been minimized.

Copy link
@instagibbs

instagibbs Feb 13, 2019

Author Member

SGTM, I like the syntax way more. So should I ping once 0.18 is branched?

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 13, 2019

Member

Remind me #14954

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 18, 2019

Member

Already broken in master, so just going to merge this as well 🤷‍♀

$ git grep -l '\.hex()' test/
test/functional/mining_basic.py
test/functional/test_framework/wallet_util.py
test/functional/wallet_txn_clone.py

This comment has been minimized.

Copy link
@instagibbs

instagibbs Feb 18, 2019

Author Member

Yeah at least partially my bad 🥇

clone_tx = CTransaction()
clone_tx.deserialize(io.BytesIO(bytes.fromhex(clone_raw)))
if (rawtx1["vout"][0]["value"] == 40 and clone_tx.vout[0].nValue != 40*COIN or rawtx1["vout"][0]["value"] != 40 and clone_tx.vout[0].nValue == 40*COIN):
(clone_tx.vout[0], clone_tx.vout[1]) = (clone_tx.vout[1], clone_tx.vout[0])

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 13, 2019

Member

nice swap

@MarcoFalke MarcoFalke added the Tests label Feb 13, 2019

clone_tx = CTransaction()
clone_tx.deserialize(io.BytesIO(bytes.fromhex(clone_raw)))
if (rawtx1["vout"][0]["value"] == 40 and clone_tx.vout[0].nValue != 40*COIN or rawtx1["vout"][0]["value"] != 40 and clone_tx.vout[0].nValue == 40*COIN):
(clone_tx.vout[0], clone_tx.vout[1]) = (clone_tx.vout[1], clone_tx.vout[0])

This comment has been minimized.

Copy link
@practicalswift

practicalswift Feb 13, 2019

Member

Nit: Parantheses redundant :-)

clone_raw = clone_raw[:pos0] + output1 + output0 + clone_raw[pos0 + 2 * output_len:]
clone_tx = CTransaction()
clone_tx.deserialize(io.BytesIO(bytes.fromhex(clone_raw)))
if (rawtx1["vout"][0]["value"] == 40 and clone_tx.vout[0].nValue != 40*COIN or rawtx1["vout"][0]["value"] != 40 and clone_tx.vout[0].nValue == 40*COIN):

This comment has been minimized.

Copy link
@practicalswift

practicalswift Feb 13, 2019

Member

Nit: Parantheses redundant :-)

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Oh yes. Thanks for doing this.
utACK 6aaa0ab once 0.18 is branched off.

@MarcoFalke MarcoFalke merged commit 6aaa0ab into bitcoin:master Feb 18, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Feb 18, 2019

Merge #15397: Remove manual byte editing in wallet_tx_clone func test
6aaa0ab Remove manual byte editing in wallet_tx_clone func test (Gregory Sanders)

Pull request description:

  Adapted from @stevenroose

Tree-SHA512: 87f251579e347f870bd30fc57b0c130f00914a3dc78799826384eb049b91d49f2525d55899bf525997e23cc976ca7d10e6b56b23f7358acec307368d48a6f6f1

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Feb 20, 2019

Merge bitcoin#15439: tests: remove byte.hex() to keep compatibility
1a062b8 tests: remove byte.hex() to keep compatibility (Akio Nakamura)

Pull request description:

  Use ```test_framework.util.bytes_to_hex_str()``` instead of ```bytes.hex()``` that new in Python 3.5 to support minimum version of Python(test).

  ```test/functional/test_framework/wallet_util.py``` is also reported to have '\.hex()' in bitcoin#15397,
  but it does not matter because it calls CScript.hex() defined in wallet_util.py.

Tree-SHA512: 1019212e965f0848d235fab4da11959dffa42e8adfcd41216c10795cfc63c804b5deb5a3317f25d29940b9dcf088ab76fe3fa80d2679dc19f5f482dc5bde3283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.