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

feat[contracts]: Restrict tx nonce and gasLimit to uint64 #620

Closed
wants to merge 1 commit into from

Conversation

smartcontracts
Copy link
Contributor

Description
Restricts nonce and gasLimit transaction fields to uint64. Also restricts the nonce field within the account struct to uint64.

@changeset-bot
Copy link

changeset-bot bot commented Apr 24, 2021

⚠️ No Changeset found

Latest commit: 135a0aa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@transmissions11
Copy link
Contributor

transmissions11 commented Apr 24, 2021

Does gasPrice have to be uint256 or could it also become uint64 (it could fit a gas price up to 50 ETH)

@maurelian
Copy link
Contributor

maurelian commented Apr 24, 2021

Is the motivation here to save gas? Have we measured the difference?
I imagine this will save gas on calldata (space), but cost more for computation during fraud proving (ex).

Edit: Oh, I see it's about compatibility with geth.

Copy link
Contributor

@maurelian maurelian left a comment

Choose a reason for hiding this comment

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

LGTM once integration tests are passing.

@smartcontracts
Copy link
Contributor Author

Closing this for now until we can confirm that we actually care about this restriction

@smartcontracts smartcontracts deleted the fix/restrict-tx-fields branch May 10, 2021 22:47
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.

3 participants