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(WETH): Add decoder for all supported EVM chains #7708
Conversation
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.
You forgot to add the changelog entry in docs/changelog.rst
b7337a8
to
90af540
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7708 +/- ##
===========================================
- Coverage 77.74% 77.32% -0.42%
===========================================
Files 949 952 +3
Lines 63400 63437 +37
Branches 11183 11187 +4
===========================================
- Hits 49288 49054 -234
- Misses 11919 12174 +255
- Partials 2193 2209 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
balance=Balance(amount=FVal('0.000001897927938075')), | ||
location_label='0x81aa5101D4c376cd6DC031EA62D7b64A9BAE10a0', | ||
notes='Burned 0.000001897927938075 ETH for gas', |
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.
To avoid repeating the values in balance and notes check what we do here
deposit_amount, gas_fees = '99503', '0.007154122119159412' |
b45ffc6
to
232d90b
Compare
a1ec89c
to
dc55616
Compare
docs/changelog.rst
Outdated
@@ -3,6 +3,7 @@ Changelog | |||
========= | |||
|
|||
* :feature:`-` rotki will now properly decode the transactions for bridging to and from Scroll. | |||
* :feature:`7708` rotki now properly decode WETH transactions on all supported EVM chains with native ETH |
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.
Seems this does not do it for non-native, but I would just add the changelog when it is all done. And not 1 changelog per entry.
* :feature:`7708` rotki now properly decode WETH transactions on all supported EVM chains with native ETH | |
* :feature:`7708` rotki now properly decodes all native to/from wrapped token transactions (e.g. ETH<->WETH, Matic<->WMatic) on all supported EVM chains |
dc55616
to
264de87
Compare
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.
Left some comments.
Also test_balancer_v1_join
that failed was a remote error. We can VCR in another PR. Do not mix here
|
||
@pytest.mark.vcr() | ||
@pytest.mark.parametrize('base_accounts', [['0xf396e7dbb20489D47F2daBfDA013163223B892a0']]) | ||
def test_weth_deposit_base(database, base_inquirer): |
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.
you already did them but there is no need to tests every case for every chain if they are just repeating the logic across other evm chains.
Just 1 test per chain if other cases are repeating.
3cabc20
to
0bab26c
Compare
event.balance.amount == deposited_amount and | ||
event.asset == self.base_asset | ||
): | ||
if event.address == depositor: |
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.
not done yet
0bab26c
to
07fdb2e
Compare
07fdb2e
to
c5d3444
Compare
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.
lgtm
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2024 rotki Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
rotki/test-caching/tree/feat/weth-evm-decoder was successfully merged |
This PR adds builtin WETH decoder for all supported EVM chains except Gnosis and Polygon PoS.
SqlDiff: