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

EIP-1559 support for web3.eth.sendTransaction #4220

Merged
merged 27 commits into from
Aug 5, 2021

Conversation

spacesailor24
Copy link
Contributor

Adds EIP-1559 support to web3.eth.sendTransaction

closes #4211

@spacesailor24 spacesailor24 added the 1.x 1.0 related issues label Aug 3, 2021
@render
Copy link

render bot commented Aug 3, 2021

payload.params[0].gasPrice = gasPrice;
}
_handleTxPricing(method, payload.params[0]).then(txPricing => {
payload.params[0] = {...payload.params[0], ...txPricing};
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make more sense to move all this logic outside of the blocks?

Regardless of how its being sent, we should just update the TX accordingly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in this case, we only want to add gas pricing info to the payload object if the info is not already provided for specifically sendTxs, otherwise the code shouldn't be executed

@coveralls
Copy link

coveralls commented Aug 4, 2021

Pull Request Test Coverage Report for Build 1099905485

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 25 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.4%) to 75.011%

Files with Coverage Reduction New Missed Lines %
packages/web3-core-method/lib/index.js 25 88.37%
Totals Coverage Status
Change from base Build 1085858469: -0.4%
Covered Lines: 3203
Relevant Lines: 4034

💛 - Coveralls

@williamsfu99
Copy link

Hi @spacesailor24, are we planning on closing this before the hardfork?

@spacesailor24
Copy link
Contributor Author

Hi @spacesailor24, are we planning on closing this before the hardfork?

@williamsfu99 yep, that's the plan, however we're having trouble getting the Ganache test suite to pass

@gnidan
Copy link
Contributor

gnidan commented Aug 4, 2021

we're having trouble getting the Ganache test suite to pass

@spacesailor24 just so you know, Ganache doesn't support London yet (and won't til Ganache v7).

@lgtm-com
Copy link

lgtm-com bot commented Aug 5, 2021

This pull request introduces 1 alert and fixes 1 when merging 3ef9420 into d0d82b4 - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Useless conditional

@spacesailor24
Copy link
Contributor Author

This is just waiting approvals now

@spacesailor24 spacesailor24 merged commit c474f8f into 1.x Aug 5, 2021
@spacesailor24 spacesailor24 deleted the wyatt/send-transaction-fix branch August 5, 2021 03:34
@spacesailor24
Copy link
Contributor Author

v1.5.1-rc.1 contains this fix FYI

Not sure when v1.5.1 will be released, probably Soon™️

@haltman-at
Copy link
Contributor

haltman-at commented Aug 5, 2021

Just saw the release candidate and tested this and can confirm that it works. :)

Edit: Also our CI with it is passing too. :)

@spacesailor24 spacesailor24 mentioned this pull request Aug 5, 2021
@spacesailor24
Copy link
Contributor Author

v1.5.1 has been released

@gnidan
Copy link
Contributor

gnidan commented Aug 5, 2021

Thank you for your tireless work getting this out, @spacesailor24

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.

EIP-1559/London/0x2 transactions do not work due to default inclusion of gasPrice
8 participants