Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

support newer versons of ethereumjs/tx with immutability #61

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

brad-decker
Copy link
Contributor

The current implementation of the sign method relies on mutation, which was the case pre version 3.0.0 of ethereumjs/tx. In earlier versions of the library, the passed tx object was modified. In version 3.0.0 immutability and Object.freeze were implemented, and the newly signed tx is returned as a brand new instance of the Transaction.

The change presented here should allow both versions (mutable and immutable) to function, preferring the immutable version.

@brad-decker brad-decker force-pushed the support-new-versions-of-ethereumjs-tx branch from 37e33b5 to fb6e9df Compare March 24, 2021 17:52
@brad-decker brad-decker marked this pull request as ready for review March 24, 2021 17:59
@brad-decker brad-decker requested a review from a team as a code owner March 24, 2021 17:59
NiranjanaBinoy
NiranjanaBinoy previously approved these changes Apr 16, 2021
Copy link

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

@brad-decker brad-decker merged commit 4e38d89 into master Apr 16, 2021
@brad-decker brad-decker deleted the support-new-versions-of-ethereumjs-tx branch April 16, 2021 21:59
Gudahtt added a commit that referenced this pull request Jul 8, 2021
This package was partially replaced by it's successor `@ethereumjs/tx`
in #61, but was left around for the unit tests to ensure the keyring
was compatible with both versions. Now that we have moved away from
using `ethereum-tx` in in MetaMask itself, we don't really need to
keep two versions of the same package around anymore.
@Gudahtt Gudahtt mentioned this pull request Jul 8, 2021
Gudahtt added a commit that referenced this pull request Jul 8, 2021
This package was partially replaced by it's successor `@ethereumjs/tx`
in #61, but was left around for the unit tests to ensure the keyring
was compatible with both versions. Now that we have moved away from
using `ethereum-tx` in in MetaMask itself, we don't really need to
keep two versions of the same package around anymore.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants