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

refactor(nexus)!: refactor to move coin locking unlocking logic from axelarnet to nexus #2186

Conversation

fish-sammy
Copy link
Contributor

  • create nexus Coin struct
  • pick up the new nexus Coin and remove axelarnet Coin

Description

Todos

  • Unit tests
  • Manual tests
  • Documentation
  • Connect epics/issues
  • Tag type of change
  • Upgrade handler

Steps to Test

Expected Behaviour

Other Notes

@fish-sammy fish-sammy changed the title AXE 5101 Refactor to move coin locking unlocking logic from axelarnet to nexus refactor(nexus)!: efactor to move coin locking unlocking logic from axelarnet to nexus Sep 23, 2024
@fish-sammy fish-sammy changed the title refactor(nexus)!: efactor to move coin locking unlocking logic from axelarnet to nexus refactor(nexus)!: refactor to move coin locking unlocking logic from axelarnet to nexus Sep 23, 2024
@fish-sammy fish-sammy force-pushed the AXE-5101-Refactor-to-move-coin-locking-unlocking-logic-from-axelarnet-to-nexus branch from ed144a3 to 92df570 Compare September 24, 2024 07:04
@fish-sammy fish-sammy force-pushed the AXE-5101-Refactor-to-move-coin-locking-unlocking-logic-from-axelarnet-to-nexus branch from 125cd59 to 80879a2 Compare September 24, 2024 19:18
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

Attention: Patch coverage is 36.06089% with 672 lines in your changes missing coverage. Please review.

Project coverage is 39.17%. Comparing base (6271ef5) to head (fd6776a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
x/axelarnet/types/mock/expected_keepers.go 6.78% 506 Missing and 2 partials ⚠️
x/nexus/types/mock/expected_keepers.go 63.33% 80 Missing and 8 partials ⚠️
x/nexus/keeper/lockable_coin.go 69.52% 21 Missing and 11 partials ⚠️
x/nexus/exported/mock/types.go 72.00% 17 Missing and 4 partials ⚠️
x/axelarnet/module.go 36.36% 5 Missing and 2 partials ⚠️
x/axelarnet/keeper/msg_server.go 76.00% 5 Missing and 1 partial ⚠️
x/nexus/exported/types.go 0.00% 3 Missing ⚠️
x/nexus/module.go 0.00% 3 Missing ⚠️
x/axelarnet/handler.go 0.00% 2 Missing ⚠️
x/axelarnet/keeper/ibc_transfer.go 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2186      +/-   ##
==========================================
- Coverage   39.89%   39.17%   -0.73%     
==========================================
  Files         373      373              
  Lines       30857    31676     +819     
==========================================
+ Hits        12310    12408      +98     
- Misses      17622    18331     +709     
- Partials      925      937      +12     

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

@fish-sammy fish-sammy marked this pull request as ready for review September 26, 2024 07:54
@fish-sammy fish-sammy requested a review from a team as a code owner September 26, 2024 07:54
app/app.go Show resolved Hide resolved
app/app.go Show resolved Hide resolved
x/nexus/keeper/lockable_coin.go Outdated Show resolved Hide resolved
app/keepers.go Outdated Show resolved Hide resolved
x/axelarnet/keeper/message_route.go Outdated Show resolved Hide resolved
x/axelarnet/keeper/msg_server.go Outdated Show resolved Hide resolved
x/nexus/exported/types.go Outdated Show resolved Hide resolved
x/nexus/keeper/lockable_coin.go Outdated Show resolved Hide resolved
x/nexus/keeper/lockable_coin.go Outdated Show resolved Hide resolved
@haiyizxx haiyizxx merged commit 2ee5604 into main Oct 4, 2024
6 of 9 checks passed
@haiyizxx haiyizxx deleted the AXE-5101-Refactor-to-move-coin-locking-unlocking-logic-from-axelarnet-to-nexus branch October 4, 2024 19:43
return err
}

err = lockableAsset.LockFrom(ctx, types.AxelarIBCAccount)
Copy link
Member

Choose a reason for hiding this comment

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

I think this will break in-transit IBC transfers that timeout or have an ack error. Those packets were sent from the escrow accounts, and so the coin would be refunded back to the escrow and the locking here will fail as a result. We can explicitly check what the packet sender was to distinguish this behaviour. But could timeout/ack error be delayed indefinitely? In that case, we might always need to have this fallback

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

Successfully merging this pull request may close these issues.

4 participants