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

eth.accounts.signTransaction: expected r & s to be treated as quantity, not data #1170

Closed
carver opened this issue Nov 13, 2017 · 14 comments · Fixed by #1258
Closed

eth.accounts.signTransaction: expected r & s to be treated as quantity, not data #1170

carver opened this issue Nov 13, 2017 · 14 comments · Fixed by #1258

Comments

@carver
Copy link
Contributor

carver commented Nov 13, 2017

According to the docs, r & s are returned from signTransaction with leading zeros, implying that they are always 32 bytes of binary data. In many other implementations that return r & s (geth, parity, ethereum-keys), they are treated as integers, without leading zeros.

I would expect r & s in web3.eth.accounts.signTransaction to follow the same pattern.

@namuyan
Copy link

namuyan commented Nov 14, 2017

https://github.com/pipermerriam/web3.py/blob/master/web3/account.py#L181..L182
Do we need to change this lines to?

            'r': HexBytes(r, variable_length=True),
            's': HexBytes(s, variable_length=True),

@carver
Copy link
Contributor Author

carver commented Nov 14, 2017

@namuyan that question belongns back on the web3.py repo, not here on the web3.js repo. I'll answer back on the pull request.

@namuyan
Copy link

namuyan commented Nov 14, 2017

oops! web3.js! sorry!

@ldn0x7dc
Copy link

ldn0x7dc commented Dec 25, 2017

Occasionally run into error:
rlp: non-canonical integer (leading zero bytes) for *big.Int, decoding into (types.Transaction)(types.txdata).R.

Seems related here.

My solution:
edit node_modules/web3-eth-accounts/src/index.js
from

    var rawTx = RLP.decode(rlpEncoded).slice(0, 6).concat(Account.decodeSignature(signature));
    var rawTransaction = RLP.encode(rawTx);

to

    var rawTx = RLP.decode(rlpEncoded).slice(0, 6).concat(Account.decodeSignature(signature));

    var trimLeadingZero = function (hex) {
      while (hex && hex.startsWith('0x00')) {
        hex = '0x' + hex.slice(4);
      }
      return hex;
    }
    rawTx[7] = trimLeadingZero(rawTx[7]);
    rawTx[8] = trimLeadingZero(rawTx[8]);

    var rawTransaction = RLP.encode(rawTx);

@frozeman
Copy link
Contributor

Could somebody post a transactions which returns leading zeros 0x00.. in the signature so i can add it to the test cases of web3.js?

@carver
Copy link
Contributor Author

carver commented Jan 11, 2018

Please Reopen

I'd like to reopen, since the individual values r and s still have single leading 0s, which was the original ask in this issue.

Key, Transaction & Expected Signature

I generated a web3py test with r & s hex values shorter than 64 hex characters: https://github.com/ethereum/web3.py/blob/master/tests/core/eth-module/test_accounts.py#L271-L308

I copied the code below, and converted the r and s ints to hex for javascript convenience. The first one was actually from the web3js docs, IIRC:

'txn, private_key, expected_raw_tx, tx_hash, r, s, v',
(
    (
        {
            'to': '0xF0109fC8DF283027b6285cc889F5aA624EaC1F55',
            'value': 1000000000,
            'gas': 2000000,
            'gasPrice': 234567897654321,
            'nonce': 0,
            'chainId': 1
        },
        '0x4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318',
        '0xf86a8086d55698372431831e848094f0109fc8df283027b6285cc889f5aa624eac1f55843b9aca008025a009ebb6ca057a0535d6186462bc0b465b561c94a295bdb0621fc19208ab149a9ca0440ffd775ce91a833ab410777204d5341a6f9fa91216a6f3ee2c051fea6a0428',
        '0xd8f64a42b57be0d565f385378db2f6bf324ce14a594afc05de90436e9ce01f60',
        '0x9ebb6ca057a0535d6186462bc0b465b561c94a295bdb0621fc19208ab149a9c',  # 31.5 bytes
        '0x440ffd775ce91a833ab410777204d5341a6f9fa91216a6f3ee2c051fea6a0428',  # 32 bytes
        37,
    ),
    (
        {
            'to': '0xF0109fC8DF283027b6285cc889F5aA624EaC1F55',
            'value': 0,
            'gas': 31853,
            'gasPrice': 0,
            'nonce': 0,
            'chainId': 1
        },
        '0x4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318',
        '0xf85d8080827c6d94f0109fc8df283027b6285cc889f5aa624eac1f558080269f22f17b38af35286ffbb0c6376c86ec91c20ecbad93f84913a0cc15e7580cd99f83d6e12e82e3544cb4439964d5087da78f74cefeec9a450b16ae179fd8fe20',
        '0xb0c5e2c6b29eeb0b9c1d63eaa8b0f93c02ead18ae01cb7fc795b0612d3e9d55a',
        '0x22f17b38af35286ffbb0c6376c86ec91c20ecbad93f84913a0cc15e7580cd9',  # 31 bytes
        '0x83d6e12e82e3544cb4439964d5087da78f74cefeec9a450b16ae179fd8fe20',  # 31 bytes
        38,
    ),
)

@frozeman
Copy link
Contributor

I just merged the issue, not new release was made. Or did you try with the 1.0 branch?

@frozeman
Copy link
Contributor

And btw, this is web3.js not py

@frozeman
Copy link
Contributor

I added a change where we make sure to add a 0 if the returned bytes are uneven, can you test if that fixes the issue for you? 746e440#diff-7bb2a20126193b9ecfe4723f83429c49R176

@carver
Copy link
Contributor Author

carver commented Jan 11, 2018

I just merged the issue, not new release was made.

Right, but that PR said fixes #1170 even though it doesn't. It fixes a slightly different issue. I commented on that thread about why.

And btw, this is web3.js not py

Yes, I know. :) I work on web3.py, and a few months ago added some local private key signing code. As part of that, I had to generate tests for this same scenario (r and s that are less than 64 hex characters). The transaction data, private key, etc should all be reusable in web3.js, though.

I added a change where we make sure to add a 0 if the returned bytes are uneven, can you test if that fixes the issue for you? 746e440#diff-7bb2a20126193b9ecfe4723f83429c49R176

I'm proposing that r and s when returned individually should be allowed to be uneven. Geth, parity, and ethereum-keys all return uneven values. I'll submit a PR.

@frozeman
Copy link
Contributor

I'll comment there, thanks!

@frozeman
Copy link
Contributor

@malissa and @jellegerbrandy can you please test the 1.0 branch for your issuer again?

@jellegerbrandy
Copy link
Contributor

my tests pass. Thanks for merging this!

@FutureOfAI
Copy link

FutureOfAI commented Jun 8, 2019

I have same problem as you, can you help me how to deal with leading zero in raw-transaction?
My raw transaction is: f86b0285012a05f2008261a8948b733353ce21ebd8eb5ffd9f49d57830942e88158769ec95a3fa70008025a04f9dd75069b51d36ec47b5bf68e6c45f8b854cf17a661e734e7a7c651240eeaca00044280475c3d355c44829ac93118b4ebe044356185c1c08724c8bcfebbd4b3d
Boardcast shows error: non-canonical integer (leading zero bytes) for *big.Int, decoding into (types.Transaction)(types.txdata).S"
including raw-transaction:
R is 4f9dd75069b51d36ec47b5bf68e6c45f8b854cf17a661e734e7a7c651240eeac
S is 0044280475c3d355c44829ac93118b4ebe044356185c1c08724c8bcfebbd4b3d
How can I deal with leading zero, and how can I re-construction raw-transaction after correct it?
Does the process like this?
RLP{ RLP(nonce) + RLP(Gas price) + RLP(Gas limit) + RLP(to address) + RLP(value) + RLP(data) + V + [a0 + FixLeadingZero(R)] + [a0 + FixLeadingZero(S)] }

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

Successfully merging a pull request may close this issue.

6 participants