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

chainId larger than 0x3fffffff messes up transaction signature recovery #2777

Closed
Fang- opened this issue May 3, 2019 · 11 comments
Closed

chainId larger than 0x3fffffff messes up transaction signature recovery #2777

Fang- opened this issue May 3, 2019 · 11 comments
Labels
1.x 1.0 related issues 2.x 2.0 related issues Bug Addressing a bug Stale Has not received enough activity

Comments

@Fang-
Copy link

Fang- commented May 3, 2019

Description

The transactions still seem to send/work fine, but recovery gives incorrect results.

Expected behavior

Correct results from recoverTransaction() regardless of transaction chainId.

Actual behavior

Correct results from recoverTransaction() for chainIds of 0x3fffffff and lower.
Incorrect results from recoverTransaction() for chainIds of 0x40000000 and higher. (Ganache defaults to timestamp as chain ID and easily shoots past this.)

Steps to reproduce the behavior

let tx = {
  chainId: "0x1",
  gasLimit: "0x927c0",
  gasPrice: "0x4a817c800",
  nonce: "0x9",
  to: "0x0000000000000000000000000000000000000000",
  value: 100
};
const pk = 'a44de2416ee6beb2f323fab48b432925c9785808d33a6ca6d7ba00b45e9370c4';
const sign = web3.eth.accounts.signTransaction;
const recover = web3.eth.accounts.recoverTransaction;

let signed = await sign(tx, pk);
console.log(recover(signed.rawTransaction));
// > 0xE2a2776777F90e4e38989486544d472bAB067158

tx.chainId = "0x3ffffffe";
signed = await sign(tx, pk);
console.log(recover(signed.rawTransaction));
// > 0xE2a2776777F90e4e38989486544d472bAB067158

tx.chainId = "0x3fffffff";
signed = await sign(tx, pk);
console.log(recover(signed.rawTransaction));
// > 0xE2a2776777F90e4e38989486544d472bAB067158

tx.chainId = "0x40000000";
signed = await sign(tx, pk);
console.log(recover(signed.rawTransaction));
// > 0x3637aa4F877204708cCC92d153648D0e1fa0149e

tx.chainId = "0x40000001";
signed = await sign(tx, pk);
console.log(recover(signed.rawTransaction));
// > 0x1e5B6F3Ca367B664148DD2F71E327daB0bC913Ad

Versions

beta.54

@nivida
Copy link
Contributor

nivida commented May 6, 2019

Thanks for opening this issue! I will test and if required fix it asap.

@nivida nivida added the Needs Clarification Requires additional input label May 6, 2019
@princesinha19
Copy link
Contributor

princesinha19 commented May 6, 2019

@Fang- @nivida I tested this and it seems to be a bug during the recovery (web3.eth.accounts.recoverTransaction) process. So, I tried to reproduce the above code using ethereumjs-tx too and that was working fine. So, One option is to use ethereumjs-tx at the time of recovery too, as we are already using ethereumjs-tx during the signing (web3.eth.accounts.signTransaction) process.

@nivida nivida added Bug Addressing a bug and removed Needs Clarification Requires additional input labels May 6, 2019
@princesinha19
Copy link
Contributor

@nivida what's your thinking here. Should we use ethereumjs-tx in recoverTransaction too?

Fang- added a commit to urbit/bridge that referenced this issue May 7, 2019
Fang- added a commit to urbit/bridge that referenced this issue May 7, 2019
@nivida nivida added the 2.x 2.0 related issues label Jun 20, 2019
@github-actions
Copy link

github-actions bot commented Jul 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 4, 2020
@Fang-
Copy link
Author

Fang- commented Jul 6, 2020

Can still reproduce on 1.2.9, fwiw. This is tagged as "2.x". Does that mean this won't be fixed in 1.x?

@ryanio ryanio added 1.x 1.0 related issues and removed Stale Has not received enough activity labels Jul 6, 2020
@ryanio
Copy link
Collaborator

ryanio commented Jul 6, 2020

@Fang- thanks for the update, I have adjusted the labels to make sure we also address this in 1.x

@github-actions
Copy link

github-actions bot commented Aug 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Aug 6, 2020
@Fang-
Copy link
Author

Fang- commented Aug 6, 2020

Bumping as not stale, considering I haven't seen this referenced as fixed yet.

@github-actions github-actions bot removed the Stale Has not received enough activity label Aug 7, 2020
@github-actions
Copy link

github-actions bot commented Oct 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Oct 6, 2020
@Fang-
Copy link
Author

Fang- commented Oct 6, 2020

Ditto.

@github-actions github-actions bot removed the Stale Has not received enough activity label Oct 7, 2020
@github-actions
Copy link

github-actions bot commented Dec 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues 2.x 2.0 related issues Bug Addressing a bug Stale Has not received enough activity
Projects
None yet
Development

No branches or pull requests

4 participants