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

signing transaction with ethereumjs/tx #344

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

signing transaction with ethereumjs/tx #344

wants to merge 8 commits into from

Conversation

sifnoc
Copy link
Member

@sifnoc sifnoc commented Dec 15, 2021 •
edited

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

For fixing #337

wanseob
wanseob requested changes Jan 10, 2022
View reviewed changes
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?

cloudinertia
cloudinertia approved these changes Jan 19, 2022
View reviewed changes
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.

vimwitch
vimwitch reviewed Jan 19, 2022
View reviewed changes
package.json Show resolved Hide resolved
@vimwitch vimwitch 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
Reviewers

@vimwitch vimwitch vimwitch approved these changes

@wanseob wanseob wanseob approved these changes

@cloudinertia cloudinertia cloudinertia approved these changes

Assignees

@sifnoc sifnoc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sifnoc @wanseob @vimwitch @cloudinertia