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

signing transaction with ethereumjs/tx #344

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

sifnoc
Copy link
Member

@sifnoc sifnoc commented Dec 15, 2021

  • added ethereumjs/common package
  • sign transaction using ethereumjs/tx instead web3.eth.account

For fixing #337

Copy link
Member

@wanseob wanseob left a comment

Choose a reason for hiding this comment

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

and please resolve lint & integration test errors :)

packages/contracts/src/tx-util.ts Show resolved Hide resolved
@wanseob wanseob requested review from a team and removed request for a team January 17, 2022 17:25
@sifnoc sifnoc self-assigned this Jan 18, 2022
@wanseob
Copy link
Member

wanseob commented Jan 19, 2022

@sifnoc I think it'll be good to add EIP1559 gas standard to the tx util

@sifnoc
Copy link
Member Author

sifnoc commented Jan 19, 2022

@sifnoc I think it'll be good to add EIP1559 gas standard to the tx util

Sure, as optional? or as default?

Copy link

@cloudinertia cloudinertia left a comment

Choose a reason for hiding this comment

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

LGTM!

@sifnoc
Copy link
Member Author

sifnoc commented Jan 19, 2022

@sifnoc I think it'll be good to add EIP1559 gas standard to the tx util

I think it is better to open new issue for supporting EIP-1559 typed transaction due to web3. The web3 version we using in zkopru currently not support also. So, we need to update web3 version then fully test with other packages.

@chancehudson chancehudson self-requested a review January 19, 2022 23:03
@wanseob
Copy link
Member

wanseob commented Feb 15, 2022

@sifnoc Hi Jin would u please resolv the conflict and merge this to the main branch?

@sifnoc
Copy link
Member Author

sifnoc commented Feb 15, 2022

@sifnoc Hi Jin would u please resolv the conflict and merge this to the main branch?

Sure!

@sifnoc
Copy link
Member Author

sifnoc commented Apr 6, 2022

This PR no longer necessary if merge #381
I will close this when it done.

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.

4 participants