Skip to content
This repository has been archived by the owner on Jun 5, 2019. It is now read-only.

Ethereum transaction signature problem #236

Closed
CryptoIR opened this issue Mar 13, 2018 · 12 comments
Closed

Ethereum transaction signature problem #236

CryptoIR opened this issue Mar 13, 2018 · 12 comments
Labels
Milestone

Comments

@CryptoIR
Copy link

Hi,
It seems like when moving to python3 the ETH signature signed by the code is wrong (at least to me).
Maybe the differences in decode/encoding methods between python2 and python3

I'm referring to this part of the code:

sig = client.ethereum_sign_tx(
        n=address_n,
        nonce=nonce,
        gas_price=gas_price,
        gas_limit=gas_limit,
        to=to_address,
        value=value,
        data=data,
        chain_id=chain_id)

    transaction = rlp.encode(
        (nonce, gas_price, gas_limit, to_address, value, data) + sig)

Something like this resolved me the issue
screen shot 2018-03-13 at 17 16 43

Did anyone else encounter it or is it just my environment?
Can you please comment?

@matejcik
Copy link
Contributor

Far as I can tell, your diff is a no-op. The result should be the same.
The signature values are byte buffers, which you're converting to int. However, rlp doesn't care about the distinction and encodes the values as bytes again.

You can check this pretty easily: duplicate the piece of code and print out tx_hex for unmodified code and after your modifications. Then see if the results differ.

For a minimal case, you can examine the result of rlp.encode(sig_r) before and after your conversion to int.

@CryptoIR
Copy link
Author

Before the conversion sig_r is a bytearray and just an integer after.
Could this be the issue when using rlp?

After rpl.encode I get completely different signature (with and without the change)
The bottom line is that the eth transaction fails without the fix and works after.

Can you also try?

@prusnak
Copy link
Member

prusnak commented Mar 13, 2018 via email

@matejcik
Copy link
Contributor

rlp is python3 compatible all right, but it wrongly differentiates between bytes and bytearray.

I'm not sure what behavior the upstream intended, but it's different between py2 and py3, hence ethereum/pyrlp#49

We could also fix this on our side by explicitly converting the bytearrays to bytes when returning the signature. As bytearrays are mutable, that might even be a good idea.

@prusnak
Copy link
Member

prusnak commented Mar 14, 2018

@matejcik Let's wait if they merge the fix and push it to PyPI (in, let's say, a few days). If not, I propose to fix it in our code like you suggest.

@matejcik
Copy link
Contributor

@prusnak I'll do some refactoring of our code in the meantime, and if it turns out I like returning bytes instead of bytearrays, I'll do that too. Better to be doubly sure :)

@prusnak
Copy link
Member

prusnak commented Mar 14, 2018

👍

@CryptoIR
Copy link
Author

Thanks guys!

@jhoenicke
Copy link
Contributor

jhoenicke commented Mar 14, 2018

A difference is that the integer variant strips leading zeros. I was always under the impression that r and s should be encoded as 32-byte buffers with leading zeros, but checking the yellow paper again it seems I was wrong. Is this a bug in the Trezor code?

It should not be too difficult to brute-force a transaction where the signature has a leading zero byte to test his.

@matejcik matejcik added this to the v0.9.2 milestone Mar 20, 2018
@matejcik matejcik added the bug label Mar 20, 2018
matejcik added a commit that referenced this issue Mar 20, 2018
…rray.

This makes more sense, because bytes are immutable and callers have no business
mutating structures from the wire anyway.

Incidentally this should fix issue #236, where rlp library would treat
bytes and bytearrays differently and produce invalid structures in our usecase.

Also very minor nitpicks and code cleanup for neater typing.
@matejcik
Copy link
Contributor

this should now be fixed on our side as well.

@purpleeggplant
Copy link

purpleeggplant commented May 31, 2018

I'm still getting the following error when trying to post signed transactions on Etherscan (http://ropsten.etherscan.io/pushtx) (Ethereum Ropsten).

Error! Unable to broadcast Tx : {"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid RLP.","data":"RlpExpectedToBeData"},"id":1}

Posting on MyEtherWallet (#offline-transaction) provides a similar error:

rlp: expected input string or byte for *big.Int, decoding into (types.Transaction)(types.txdata).R

As far as I can tell, all my dependencies are good. I'm using a Raspberry Pi3. I am able to complete this operation successfully on MacOs computer without issues. The main difference i see is the MacBook is running Eth-Hash 0.1.2 instead of 0.1.4 (on the Rasp Pi). Although at second glance, it looks like ethereum reverted back from 2.3.1 to 1.0.8.

@purpleeggplant
Copy link

purpleeggplant commented May 31, 2018

manually installing eth-hash to 0.1.2 and ethereum to 2.3.1 doesn't help. All the packages are the same yet running the exact same command outputs two different Signed Raw Transactions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants