-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add AccessList to 1559 transaction rlp encoding #1992
Conversation
Hey @liewhite your PR is really useful, in order to can merge it could you please fix the build issues by fixing the tests? thanks |
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1992 +/- ##
============================================
- Coverage 68.69% 68.61% -0.09%
+ Complexity 3041 3040 -1
============================================
Files 490 490
Lines 12715 12726 +11
Branches 1653 1653
============================================
- Hits 8735 8732 -3
- Misses 3497 3509 +12
- Partials 483 485 +2 ☔ View full report in Codecov by Sentry. |
return chainId; | ||
@Override | ||
public BigInteger getGasPrice() { | ||
throw new UnsupportedOperationException("not available for 1559 transaction"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be nice to have a test case for this
super(chainId, nonce, null, gasLimit, to, value, data, Collections.emptyList()); | ||
this.maxPriorityFeePerGas = maxPriorityFeePerGas; | ||
this.maxFeePerGas = maxFeePerGas; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it looks simple a test case will be worth here, also will improve the test coverage
What does this PR do?
Add AccessList to 1559 transaction
Where should the reviewer start?
org.web3j.crypto.transaction.type.Transaction1559
Why is it needed?
#1991
It's an omission in web3j