tx: Fix EIP-155 transaction encoding on chain ID 0. #2671
Conversation
When preparing Ethereum data for RLP encoding, you must convert all values to either byte arrays or lists. Scalar values (positive integers) are converted to byte arrays by using the fewest bytes possible to represent the value in big endian format. The canonical form of any scalar value prepped for RLP encoding is that any leading `0` bytes stripped. In the case of `0`, this means that the only byte (`0x00`) is stripped and you are left with an empty array. Most of the values in this code are properly 0-stripped using `bigIntToUnpaddedBuffer`. Unfortunately, it appears that the `chainId` was added without stripping leading zeros. In most cases this doesn't matter, but if someone is running a chainId 0 blockchain and they want to EIP-155 encode their transaction, the previous code would result in an invalid transaction because only the canonical form is allowed. This change fixes this bug and correctly prepares the chainId in EIP-155 transactions for RLP encoding.
MicahZoltu
changed the title
g11tech
changed the title
g11tech
added
PR state: merge ready
package: tx
target: master
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2023 EthereumJS Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
|
Just to note: we have somewhat of a So we might take this change over to our (@MicahZoltu or is this an urgent bug fix for you?) |
|
It is not an urgent bugfix. The ethers test suite used this library to generate its test cases which resulted in a large number of test cases that are invalid/incorrect, and I ran across it when testing my library against that suite. Getting this fixed would let ethers fix its test suite which would let me test my library more completely. 😄 |
When preparing Ethereum data for RLP encoding, you must convert all values to either byte arrays or lists. Scalar values (positive integers) are converted to byte arrays by using the fewest bytes possible to represent the value in big endian format. The canonical form of any scalar value prepped for RLP encoding is that any leading
0bytes stripped. In the case of0, this means that the only byte (0x00) is stripped and you are left with an empty array.Most of the values in this code are properly 0-stripped using
bigIntToUnpaddedBuffer. Unfortunately, it appears that thechainIdwas added without stripping leading zeros. In most cases this doesn't matter, but if someone is running a chainId 0 blockchain and they want to EIP-155 encode their transaction, the previous code would result in an invalid transaction because only the canonical form is allowed.This change fixes this bug and correctly prepares the chainId in EIP-155 transactions for RLP encoding.