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

Support Ethers 0.6.0+ #28

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Support Ethers 0.6.0+ #28

merged 2 commits into from
Jan 7, 2025

Conversation

alisinabh
Copy link
Member

@alisinabh alisinabh commented Jan 2, 2025

Ethers 0.6.0 was released with several breaking changes that needs to be addressed in order for KMS signer to keep working with it. The changes that impacted KMS signer are related to the newly introduced transaction structs (e.g. Eip1559 and Legacy).

These changes are not backwards compatible as some requirements around transactions has changed. Older versions of Ethers can keep using version 0.0.3 of this library.

No further user action is required other than making sure their codebase is compatible with Ethers 0.6.0+.

I suggest at least a minor version bump.

@alisinabh alisinabh self-assigned this Jan 2, 2025
@wchenNL wchenNL self-requested a review January 7, 2025 14:53
Copy link
Contributor

@wchenNL wchenNL left a comment

Choose a reason for hiding this comment

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

LGTM, although interesting to see from is removed from the transaction struct and now has to be passed in as a new param.

It looks like a major release to me since it's not backward compatible.

@alisinabh
Copy link
Member Author

LGTM, although interesting to see from is removed from the transaction struct and now has to be passed in as a new param.

This is to reflect the actual transaction structure in EVM chains. The from address is a byproduct of signature and does not exist in an unsigned transaction payload. But this is taken care of in the new version of Ethers with the Transaction.Signed module.

It looks like a major release to me since it's not backward compatible.

One thing to note is that from user's perspective (public API) this change is transparent. As long as they conform to Ethers 0.6+ (which is enforced by this libraries mix.exs) they should be good to go. I'm personally okay with a minor or even a major release. Just not a patch one.

@wchenNL
Copy link
Contributor

wchenNL commented Jan 7, 2025

LGTM, although interesting to see from is removed from the transaction struct and now has to be passed in as a new param.

This is to reflect the actual transaction structure in EVM chains. The from address is a byproduct of signature and does not exist in an unsigned transaction payload. But this is taken care of in the new version of Ethers with the Transaction.Signed module.

It looks like a major release to me since it's not backward compatible.

One thing to note is that from user's perspective (public API) this change is transparent. As long as they conform to Ethers 0.6+ (which is enforced by this libraries mix.exs) they should be good to go. I'm personally okay with a minor or even a major release. Just not a patch one.

👍 Minor sounds reasonable to me then.

@wchenNL wchenNL merged commit eb1b9be into main Jan 7, 2025
4 checks passed
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