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.

Sign up for GitHub

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

Jump to bottom

feat: Add decoders for Hop Protocol Bridge #7911

Merged
merged 1 commit into from
May 17, 2024
Merged

feat: Add decoders for Hop Protocol Bridge #7911

merged 1 commit into from
May 17, 2024

Conversation

moxw
Copy link
Contributor

@moxw moxw commented May 9, 2024 •
edited
Loading

Relates to #7202

@moxw moxw added the ready for peer review Backend PR ready to be reviewed by colleagues label May 9, 2024
@moxw moxw force-pushed the decode-hop-bridge branch 3 times, most recently from ed83e31 to 5ec528f Compare May 9, 2024 08:24
yabirgb
yabirgb reviewed May 9, 2024
View reviewed changes
Copy link
Member

@yabirgb yabirgb left a comment

Choose a reason for hiding this comment

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

The PR doesn't close the issue as it still needs to handle LPs. Please remove the closes # from the comment

rotkehlchen/chain/arbitrum_one/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/arbitrum_one/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/arbitrum_one/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/base/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/base/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/gnosis/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/optimism/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/polygon_pos/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/constants/assets.py Outdated Show resolved Hide resolved
@codecov Codecov
Copy link

codecov bot commented May 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 77.17391% with 63 lines in your changes are missing coverage. Please review.

Project coverage is 80.48%. Comparing base (81757fe) to head (7b6289b).
Report is 6 commits behind head on develop.

Files Patch % Lines
rotkehlchen/chain/evm/decoding/hop/decoder.py 74.35% 22 Missing and 18 partials ⚠️
rotkehlchen/chain/ethereum/modules/hop/decoder.py 68.18% 4 Missing and 3 partials ⚠️
...kehlchen/chain/arbitrum_one/modules/hop/decoder.py 60.00% 3 Missing and 1 partial ⚠️
rotkehlchen/chain/base/modules/hop/decoder.py 60.00% 3 Missing and 1 partial ⚠️
rotkehlchen/chain/gnosis/modules/hop/decoder.py 60.00% 3 Missing and 1 partial ⚠️
...tkehlchen/chain/polygon_pos/modules/hop/decoder.py 60.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #7911      +/-   ##
===========================================
- Coverage    80.74%   80.48%   -0.26%     
===========================================
  Files         1168     1181      +13     
  Lines       107797   107993     +196     
  Branches     12837    12868      +31     
===========================================
- Hits         87039    86919     -120     
- Misses       18545    18818     +273     
- Partials      2213     2256      +43
Flag Coverage Δ
backend 80.82% <77.17%> (-0.53%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@moxw moxw requested a review from yabirgb May 9, 2024 17:06
@moxw moxw force-pushed the decode-hop-bridge branch 2 times, most recently from 95c6b71 to bf9e751 Compare May 9, 2024 18:59
yabirgb
yabirgb reviewed May 10, 2024
View reviewed changes
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/constants/assets.py Outdated Show resolved Hide resolved
@moxw moxw force-pushed the decode-hop-bridge branch 3 times, most recently from 1142c91 to 236dd4c Compare May 10, 2024 14:38
yabirgb
yabirgb reviewed May 10, 2024
View reviewed changes
rotkehlchen/chain/base/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/ethereum/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/constants/assets.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
@moxw moxw force-pushed the decode-hop-bridge branch 4 times, most recently from f2c3b07 to d45573b Compare May 14, 2024 05:40
yabirgb
yabirgb reviewed May 14, 2024
View reviewed changes
rotkehlchen/chain/ethereum/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/types.py Outdated Show resolved Hide resolved
rotkehlchen/chain/polygon_pos/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
@yabirgb
Copy link
Member

yabirgb commented May 14, 2024

image
You haven't done that

@yabirgb yabirgb added PR review work work on PR review comments and removed ready for peer review Backend PR ready to be reviewed by colleagues labels May 14, 2024
@moxw
Copy link
Contributor Author

moxw commented May 14, 2024

image You haven't done that

I had it changed to "relates", it's still there.

@yabirgb yabirgb added ready for peer review Backend PR ready to be reviewed by colleagues and removed PR review work work on PR review comments labels May 14, 2024
yabirgb
yabirgb reviewed May 14, 2024
View reviewed changes
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Show resolved Hide resolved
@moxw moxw force-pushed the decode-hop-bridge branch 2 times, most recently from bc70cc5 to f57d232 Compare May 16, 2024 03:00
@yabirgb yabirgb added ready for final review Backend PR ready to be reviewed by great Lefteris and removed ready for peer review Backend PR ready to be reviewed by colleagues labels May 16, 2024
LefterisJP
LefterisJP reviewed May 16, 2024
View reviewed changes
Copy link
Member

@LefterisJP LefterisJP left a comment

Choose a reason for hiding this comment

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

Lots of comments. Also in the tests I saw no test of a bridging with a different target address to see that part ofthe code work. We need one. Find and test it.

rotkehlchen/chain/arbitrum_one/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/arbitrum_one/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/ethereum/modules/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Show resolved Hide resolved
@LefterisJP LefterisJP added PR review work work on PR review comments and removed ready for final review Backend PR ready to be reviewed by great Lefteris labels May 16, 2024
@moxw moxw force-pushed the decode-hop-bridge branch 8 times, most recently from cd4dccd to 292d7a5 Compare May 17, 2024 14:16
@moxw moxw added ready for final review Backend PR ready to be reviewed by great Lefteris and removed PR review work work on PR review comments labels May 17, 2024
@moxw moxw requested a review from LefterisJP May 17, 2024 15:49
LefterisJP
LefterisJP reviewed May 17, 2024
View reviewed changes
rotkehlchen/chain/arbitrum_one/modules/hop/constants.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Outdated Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/decoder.py Show resolved Hide resolved
rotkehlchen/chain/evm/decoding/hop/structures.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Show resolved Hide resolved
LefterisJP
LefterisJP reviewed May 17, 2024
View reviewed changes
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
rotkehlchen/tests/unit/decoders/test_hop.py Outdated Show resolved Hide resolved
@moxw moxw force-pushed the decode-hop-bridge branch 3 times, most recently from 6224633 to 97fb1bc Compare May 17, 2024 20:46
@moxw @yabirgb
feat: Add decoders for Hop Protocol Bridge
7b6289b 
Co-authored-by: Yábir Benchakhtir <yabirg@protonmail.com>
LefterisJP
LefterisJP approved these changes May 17, 2024
View reviewed changes
Copy link
Member

@LefterisJP LefterisJP left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you. This was a big PR.

@LefterisJP LefterisJP merged commit 6644849 into rotki:develop May 17, 2024
13 checks passed
@rotkibot
Copy link

rotki/test-caching/tree/decode-hop-bridge was successfully merged

@moxw moxw deleted the decode-hop-bridge branch May 28, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yabirgb yabirgb yabirgb left review comments

@LefterisJP LefterisJP LefterisJP approved these changes

Assignees
No one assigned
Labels
ready for final review Backend PR ready to be reviewed by great Lefteris
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@moxw @yabirgb @rotkibot @LefterisJP