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.

Sign up for GitHub

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

Jump to bottom

contracts: Added EIP-155 signer and extended EthereumUtils #154

Merged
merged 10 commits into from Jul 17, 2023
Merged

contracts: Added EIP-155 signer and extended EthereumUtils #154

merged 10 commits into from Jul 17, 2023

Conversation

CedarMist
Copy link
Member

@CedarMist CedarMist commented Jul 16, 2023 •
edited

This allows contracts to sign and encode Ethereum compatible EIP-155 transactions.

As a follow-up to oasisprotocol/demo-voting#9 where we should make this code commonly available in sapphire-contracts library.

Truffle tests are still intermittent with callback related errors (weeiird).

This will require publishing an updated sapphire-contracts NPM repo.

RLPWriter contract is MIT licensed from elsewhere, with (multiple) attributions in the file.

TODO:

  • Commit v0.2.3 sapphire-contracts version
  • Add tests for EIP-155 oroborus

@CedarMist CedarMist self-assigned this Jul 16, 2023
@CedarMist CedarMist requested review from matevz and nhynes July 16, 2023 16:06
@CedarMist CedarMist marked this pull request as ready for review July 17, 2023 00:36
nhynes
nhynes approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@nhynes nhynes left a comment

Choose a reason for hiding this comment

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

I left a few comments that I expect will be resolved in some way before merging. Otherwise LGTM

contracts/contracts/EIP155Signer.sol Outdated Show resolved Hide resolved
contracts/contracts/EthereumUtils.sol Outdated Show resolved Hide resolved
contracts/contracts/EthereumUtils.sol Outdated Show resolved Hide resolved
contracts/contracts/RLPWriter.sol Show resolved Hide resolved
contracts/test/eip155.ts Outdated Show resolved Hide resolved
contracts/test/eip155.ts Outdated Show resolved Hide resolved
contracts/test/eip155.ts Outdated Show resolved Hide resolved
contracts/test/eip155.ts Outdated Show resolved Hide resolved
@CedarMist CedarMist merged commit a970b6e into main Jul 17, 2023
14 checks passed
@CedarMist CedarMist deleted the CedarMist/EIP-155 branch July 17, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhynes nhynes nhynes approved these changes

@matevz matevz Awaiting requested review from matevz

Assignees

@CedarMist CedarMist

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@CedarMist @nhynes