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

EIP1559 + EIP2718 support #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sidhujag
Copy link

Integrate transaction/receipt envelop's + add tests (support 1559 payloads, 2930 payloads and legacy payloads)
Integrate London block/transaction fields

Integrate transaction/receipt envelop's + add tests (support 1559 payloads, 2930 payloads and legacy payloads)
Integrate London block/transaction fields
@sidhujag
Copy link
Author

There is some related incomplete work here that is related: https://github.com/BeamMW/eth-object and https://github.com/aurora-is-near/eth-object

Comment on lines +51 to +56
]
// https://eips.ethereum.org/EIPS/eip-1559
if(rpcResult.baseFeePerGas){
data.push(toBuffer(rpcResult.baseFeePerGas));
}
return new this(data)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good here

Comment on lines 6 to 18
static get fields(){ return [
'chainId',
'nonce',
'gasPrice',
'maxPriorityFeePerGas',
'maxFeePerGas',
'gasLimit',
'to',
'value',
'data',
'accessList',
'v',
'r',
's',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a problem here because these new fields knock the ordering off when generating objects that dont include them

Copy link
Author

@sidhujag sidhujag Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this code on proofs using eth mainnet which has a collection of legacy/version 1/version 2 transactions, it does create properly, can you give example where it would break?

Comment on lines +64 to +67
let transaction = Transaction.fromHex('0x02f8a902850df847580082cd1b947c5a0ce9267ed19b22f8cae653f198e3e8daf09880b844a9059cbb00000000000000000000000083335e0c01afac5e02ff201ba0f5979ebc4aa93f00000000000000000000000000000000000000000000000340aad21b3b70000025a087e52ad60613b36f6d6dda62ff7aef013fc91340f6511c834a28f742fcfebf34a05e1bd17543f7e875e8872a3a19386e6428b661329e1ebdd38ad3fd14c256b196')
console.log("\n\ntransaction\n", transaction)
console.log("\n\ntransaction.object\n", transaction.object)
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont believe the data is being encoded properly when building a new object because your test is outputting this:

transaction.object
 {
  chainId: '0x02',
  nonce: '0x0df8475800',
  maxPriorityFeePerGas: '0xcd1b',
  maxFeePerGas: '0x7c5a0ce9267ed19b22f8cae653f198e3e8daf098',
  gasLimit: '0x',
  to: '0xa9059cbb00000000000000000000000083335e0c01afac5e02ff201ba0f5979ebc4aa93f00000000000000000000000000000000000000000000000340aad21b3b700000',
  value: '0x25',
  data: '0x87e52ad60613b36f6d6dda62ff7aef013fc91340f6511c834a28f742fcfebf34',
  accessList: '0x5e1bd17543f7e875e8872a3a19386e6428b661329e1ebdd38ad3fd14c256b196',
  v: '0x',
  r: '0x',
  s: '0x'
}

Look at the 'to' address. Its not the right number of digits. Some of the other data here also looks wrong

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats odd I will look into this looks like all the fields are messed up

Comment on lines +7 to 18
'chainId',
'nonce',
'gasPrice',
'maxPriorityFeePerGas',
'maxFeePerGas',
'gasLimit',
'to',
'value',
'data',
'accessList',
'v',
'r',
's',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'll need to have some conditionals here for backwards compatibility.

Copy link
Author

@sidhujag sidhujag Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it only encode fields that are set in the object and skips over the ones that aren't? The proofs are valid across all the tx types.

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 this pull request may close these issues.

2 participants