Manually follow EIP-155 in certain circumstances #26
Conversation
Wait, don't we still give the user the "what network" dropdown when in offline mode? We should, because it affects the transaction as you've discovered. |
|
Much obliged for fixing my issue! |
|
@juped No sweat! |
@Fang- Making a separate issue for this |
Ledger does not seem to handle EIP-155 automatically. When using a Ledger, if the block number is at least FORK_BLKNUM = 2675000, one needs to pre-set the ECDSA signature parameters with r = 0, s = 0, and v = chainId prior to signing. The easiest way to handle this on our end is to just branch on the network type, since mainnet and Ropsten have obviously passed FORK_BLKNUM. This is somewhat awkward when dealing with offline transactions, since we might want to test them on a local network as well. For now we can assume that users generating offline transactions wish to later submit them to mainnet, but the best thing to do is probably to add an 'advanced' tab to offline transaction generation where one can disable the defaulted-on EIP-155 settings in this case. See: See LedgerHQ/ledgerjs#43 (comment) https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md
At present these always point to mainnet, so are invalid when using Ropsten, local nodes, etc. This commit fixes things so that the Etherscan links are disabled, or are just not visible, when using a network other than Ropsten / mainnet, and ensures that the links go to the right place when using Ropsten. Additionally, this fixes a bug where some links were using an old 'disabled' property instead of 'prop-disabled', and so were enabled when they shouldn't be (including on mainnet).
|
This got out of sync with master, so I've rebased and force-pushed. Should be good to go again. I've also tacked on a fix for some issues around Etherscan links. |
|
Bridge crashes when I try to sign the tx tro set my networking keys on localnet with my Ledger. Screencap below:
|
This must have somehow fallen out in a rebase or merge.
|
@iamwillkim As always, nice catch. That was the previous Ledger fix which must have fallen out in a merge -- ditching the form components in #24 required a lot of manual stitching. We should probably devote some effort to adding proptypes/tests at this point. The Will Kim test suite continues to be extremely effective, but is no doubt expensive to run. |
|
Hear hear! I'll make it an issue. In other news, this PR looks good to me. |
Resolves #12.
Ledger does not seem to handle EIP-155 automatically. When using a Ledger, if the block number is at least FORK_BLKNUM = 2675000, one needs to pre-set the ECDSA signature parameters with r = 0, s = 0, and v = chainId prior to signing.
The easiest way to handle this on our end is to just branch on the network type, since mainnet and Ropsten have obviously passed FORK_BLKNUM.
This is somewhat awkward when dealing with offline transactions, since we might want to test them on a local network as well. For now we can assume that users generating offline transactions wish to later submit them to mainnet, but the best thing to do is probably to add an 'advanced' tab to offline transaction generation where one can disable the defaulted-on EIP-155 settings in that case.
See:
LedgerHQ/ledgerjs#43 (comment)
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md