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

Format transaction.type to hex. Add empty accessList is tx.type === '0x1' #5979

Merged
merged 10 commits into from
Apr 4, 2023

Conversation

spacesailor24
Copy link
Contributor

@spacesailor24 spacesailor24 commented Apr 1, 2023

  • transaction.type is now formatted to a hex string before being sent to provider
  • When sending a transaction, if transaction.type === '0x1' && transaction.accessList === 'undefined', then transaction.accessList is set to []

Added test cases for Steps to Reproduce examples from here. With these changes everything is formatted as expected, however, we do not throw an error for tx.type exceededing bounds of 0x0 to 0x7f

closes #4597

@spacesailor24 spacesailor24 added the 1.x 1.0 related issues label Apr 1, 2023
@spacesailor24 spacesailor24 self-assigned this Apr 1, 2023
@coveralls
Copy link

coveralls commented Apr 1, 2023

Pull Request Test Coverage Report for Build 4585263643

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 278 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+2.5%) to 75.042%

Files with Coverage Reduction New Missed Lines %
packages/web3-core-requestmanager/src/jsonrpc.js 2 64.29%
packages/web3-core-method/lib/index.js 5 93.92%
packages/web3-core-helpers/src/formatters.js 9 83.86%
packages/web3-core-helpers/src/errors.js 31 1.47%
packages/web3-utils/src/soliditySha3.js 34 3.43%
packages/web3-utils/src/index.js 40 40.3%
packages/web3-utils/src/utils.js 46 10.39%
packages/web3-eth-accounts/src/index.js 111 22.94%
Totals Coverage Status
Change from base Build 4571302347: 2.5%
Covered Lines: 3341
Relevant Lines: 4200

💛 - Coveralls

@spacesailor24 spacesailor24 marked this pull request as ready for review April 1, 2023 22:46
@spacesailor24 spacesailor24 changed the title Format transaction.type to hex. Add empty accessList is `tx.type === '0x1' Format transaction.type to hex. Add empty accessList is tx.type === '0x1' Apr 1, 2023
Copy link
Contributor

@jdevcs jdevcs left a comment

Choose a reason for hiding this comment

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

lgtm, except tiny refactoring.

@spacesailor24
Copy link
Contributor Author

lgtm, except tiny refactoring.

@jdevcs Merged, thank you

avkos
avkos previously requested changes Apr 3, 2023
CHANGELOG.md Outdated
### Changed

- `transaction.type` is now formatted to a hex string before being send to provider (#5979)
- When sending a transaction, if `transaction.type === '0x1' && transaction.accessList === []`, then `transaction.accessList` is set to `[]` (#5979)
Copy link
Contributor

Choose a reason for hiding this comment

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

do you mean
transaction.type === '0x1' && transaction.accessList is undefined then transaction.accessList is set to []
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated via this commit, thank you

CHANGELOG.md Outdated Show resolved Hide resolved
@spacesailor24 spacesailor24 requested a review from avkos April 3, 2023 19:51
@spacesailor24 spacesailor24 merged commit 4e5afa1 into 1.x Apr 4, 2023
This was referenced Feb 23, 2024
@coveralls
Copy link

coveralls commented Sep 30, 2024

Pull Request Test Coverage Report for Build 4600732589

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.06%) to 75.035%

Files with Coverage Reduction New Missed Lines %
packages/web3-core-method/lib/index.js 4 93.9%
Totals Coverage Status
Change from base Build 4585581023: 0.06%
Covered Lines: 3340
Relevant Lines: 4199

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad input interpretation for type parameter to transaction; also exact type is not always respected
6 participants