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

Handle migrated celo transactions #150

Merged
merged 15 commits into from
Jun 21, 2024

Conversation

piersy
Copy link

@piersy piersy commented Jun 20, 2024

Introduces support for encoding/decoding legacy celo transaction types.

That covers:

  • Type 0 - The legacy transaction has been updated and now has the celo specific fields added
  • Type 124 - The v1 of the dynamic fee tx.

Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.

piersy added 12 commits June 20, 2024 19:04
Since we will be supporting decoding and encoding all old celo tx types in op-geth we may as well use the same naming.
This means most of the existing codebase should work now
This adds the structure and rlp encode/decode functionality but no
signing or json marshaling logic because for syncing it may not be
required.
Copy link

@palango palango left a comment

Choose a reason for hiding this comment

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

Great PR, with nice incremental commits which made reviewing a breeze.

Looks great, mostly some nits about copyright headers and adding docstrings in a few places 🎉

core/types/celo_extensions.go Outdated Show resolved Hide resolved
core/types/legacy_tx_marshalling.go Outdated Show resolved Hide resolved
GasPrice *big.Int // wei per gas
Gas uint64 // gas limit

// Celo-specific fields
Copy link

Choose a reason for hiding this comment

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

Any specific reason why you put this between the ethereum fields?

Copy link
Author

Choose a reason for hiding this comment

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

@mariano made this change - 57109af

Copy link

Choose a reason for hiding this comment

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

I didn't 🤓

Copy link
Author

Choose a reason for hiding this comment

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

@mariano my apologies! @mcortesi made the change

core/types/celo_dynamic_fee_tx_v2.go Outdated Show resolved Hide resolved
core/types/celo_transaction.go Show resolved Hide resolved
@palango
Copy link

palango commented Jun 21, 2024

Oh, and please create a follow-up issue to add some tests.

@piersy
Copy link
Author

piersy commented Jun 21, 2024

Oh, and please create a follow-up issue to add some tests.

Added issue here - #151

@piersy piersy merged commit eb460e4 into celo6 Jun 21, 2024
7 checks passed
@piersy piersy deleted the piersy/handle-migrated-celo-transactions branch June 21, 2024 16:10
karlb pushed a commit that referenced this pull request Jul 10, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Jul 10, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Jul 12, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Aug 20, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Aug 26, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Aug 30, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
karlb pushed a commit that referenced this pull request Oct 11, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
piersy added a commit that referenced this pull request Oct 15, 2024
Introduces support for encoding/decoding legacy celo transaction types.

That covers:

Type 0 - The legacy transaction has been updated and now has the celo specific fields added
Type 124 - The v1 of the dynamic fee tx.
Additionally the transaction code has been updated to be less invasive to the op-geth repo by moving celo code into celo specific files and by removing celo specific methods added to the TxData interface.

Additionally I reversed the EthCompatible boolean field since it's used to determine how to encode a legacy tx, and if unset (I.E. EthCompatible == false) then the legacy tx would be encoded as a celo legacy tx, which of course broke a lot of tests. So now the field is called CeloLegacy and if unset the tx will be encoded as an eth compatible transaction.
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.

3 participants