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

feat: Onboarding IBC middleware #103

Merged
merged 38 commits into from
Aug 6, 2023

Conversation

poorphd
Copy link
Contributor

@poorphd poorphd commented Apr 27, 2023

Description

IBC onboarding middleware for Canto Onboarding UX Improvement

When users transfer assets to the Canto network through Gravity Bridge, the IBC transfer automatically triggers swap and conversion to Canto ERC20 via IBC middleware. Auto swap is triggered based on the recipient's Canto balance, and the remaining transferred balance is converted to Canto ERC20. The conversion process is independent of the result of the swap. It attempts to convert all assets transferred via IBC to ERC20 tokens, provided that the transferred asset is registered in the ERC20 token pairs. This applies even if the swap fails or is not triggered.
onboarding middleware always returns original ACK from previous middleware stack meaning that even if the swap or conversion fails, it does not revert IBC transfer and the asset transferred to the Canto network will still remain in the Canto network

For auto swap function, Coinswap module is also added.

This module is a forked modification of IRISNET's Coinswap module v1.6. Several modifications have been made to ensure that the liquidity pools are only used for onboarding. The modifications are as follows:

  • Remove double swap functionality
  • Limit the quantity of max standard coin (Canto) in the liquidity pool
  • Limit the quantity of swappable coins

IBC Testing package issue

There have been issues with onboarding middleware testing due to the low version of IBC-go currently used in Canto. When using ibc-go/testing, there were cases where EVM configuration loading failed because the proposer was not set in the block header, or the EVM call failed due to gas shortage caused by a low DefaultGenTxGas value. To resolve these issues, a custom version of ibc-go/testing is implemented in ibc/testing and used.

Tasks

  • Add onboarding module
  • Add coinswap module
  • Write unit tests and integration tests
  • Package name change v6 -> v7
  • Add v7 upgrade handler
  • Testing with Canto local chain
  • Testing with mainnet state based local chains

After this pull request is merged, a follow-up pull request will be created to change the package name from v6 to v7.

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • included the necessary unit and integration tests
  • reviewed "Files changed" and left comments if necessary

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

Resolving Security Audit Findings

All security audit findings are resolved in this PR.

@poorphd poorphd changed the title Onboarding middleware feat: Onboarding IBC middleware Apr 27, 2023
@poorphd poorphd marked this pull request as ready for review April 28, 2023 09:46
@poorphd
Copy link
Contributor Author

poorphd commented Apr 28, 2023

@zscole Please review the PR. Thanks.

Copy link
Member

@tkkwon1998 tkkwon1998 left a comment

Choose a reason for hiding this comment

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

Some changes required regarding module versioning. Canto is currently on v5, and the next state-breaking upgrade should be v6.

app/app.go Show resolved Hide resolved
app/app.go Show resolved Hide resolved
app/app.go Show resolved Hide resolved
app/upgrades/v7/constants.go Show resolved Hide resolved
app/upgrades/v7/constants.go Show resolved Hide resolved
app/upgrades/v7/upgrades.go Show resolved Hide resolved
app/upgrades/v7/upgrades_test.go Show resolved Hide resolved
@poorphd poorphd requested a review from tkkwon1998 May 9, 2023 23:04
Copy link
Member

@tkkwon1998 tkkwon1998 left a comment

Choose a reason for hiding this comment

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

Approved!

@poorphd
Copy link
Contributor Author

poorphd commented Jul 20, 2023

With the recent v6 upgrade, there are conflicts that need to be resolved in the onboarding PR.

#106
It looks like we may have to repeat the conflict resolve for the onboarding module if there is an upgrade coming soon that removes the vesting module.
@tkkwon1998 Can you give us an idea of the features that will be included in the v7 upgrade?

  • Option 1: Remove modules in v7 + onboarding merge
  • Option 2: Remove module from v7 + onboarding merge in v8

Once we have confirmation of the above, we will modify the upgrade handler to the version that will include onboarding.

@tkkwon1998
Copy link
Member

Let's go with option 1, remove modules in v7 + onboarding merge.

I'll merge #106, then we can go ahead with resolving conflicts for onboarding PR.

poorphd and others added 3 commits July 24, 2023 18:35
…re-conflict-resolve

# Conflicts:
#	app/app.go
#	app/upgrades/v6/upgrades.go
…resolve

Onboarding middleware conflict resolve
@tkkwon1998
Copy link
Member

@poorphd #109 has been merged into main branch now

@poorphd
Copy link
Contributor Author

poorphd commented Jul 31, 2023

We will resolve the conflicts once this PR is merged.

poorphd and others added 7 commits August 3, 2023 16:54
…leware-conflict-resolve

# Conflicts:
#	app/app.go
#	x/recovery/keeper/ibc_callbacks.go
#	x/recovery/keeper/ibc_callbacks_integration_suite_test.go

(cherry picked from commit 4d45df7)
…re-conflict-resolve

# Conflicts:
#	app/app.go
#	app/app_test.go
#	app/test_helpers.go

(cherry picked from commit ec76588)
(cherry picked from commit 5ac28de)
…ng-middleware-conflict-resolve

* commit 'd780d52648475e8887cacce73acec93ae59fb34b':
  chore: add branches rule for workflow
  fix: simulation seed randomness was removed to make the ci/cd result deterministic
  fix: remove recovery from storeKeysPrefixes
  add placeholder for ICS4wrapper in transferKeeper init
  remove recovery imports in app.go
  remove recovery proto
  remove x/recovery
  fix: update simulation target modules, refactor duplicated struct
  fix: removing unused keeper from handler options
  fix: simulation workflow, update sims.yml, add GOPATH on makefile
  fix: simulation errors
  fix: seperate antehandler for simulation
  feat: add workflow to build, test, codecov for ci/cd

# Conflicts:
#	app/app.go
#	app/test_helpers.go
#	proto/canto/onboarding/v1/query.proto
#	x/onboarding/client/cli/query.go
#	x/onboarding/genesis.go
#	x/onboarding/genesis_test.go
#	x/onboarding/ibc_middleware.go
#	x/onboarding/keeper/grpc_query.go
#	x/onboarding/keeper/grpc_query_test.go
#	x/onboarding/keeper/keeper.go
#	x/onboarding/keeper/keeper_test.go
#	x/onboarding/keeper/params.go
#	x/onboarding/keeper/utils_test.go
#	x/onboarding/module.go
#	x/onboarding/types/errors.go
#	x/onboarding/types/genesis.go
#	x/onboarding/types/genesis.pb.go
#	x/onboarding/types/genesis_test.go
#	x/onboarding/types/keys.go
#	x/onboarding/types/params_test.go
#	x/onboarding/types/query.pb.go
#	x/onboarding/types/query.pb.gw.go
…resolve

Onboarding middleware conflict resolve
@dongsam
Copy link
Collaborator

dongsam commented Aug 4, 2023

The update related to swagger will be carried out after the #112 is reviewed/merged or will be updated together on the PR depending on the merge order

@tkkwon1998
Copy link
Member

@dongsam #112 has been reviewed & merged now

* commit '3ea39d93546596c018f88d1802e91fbc8645ff76':
  docs: update swagger, statik
  fix: re-generate proto pb.go files
  chore: formatting proto files
  build: fix proto gen, swagger script
  docs: remove deprecated module's on swagger config
  chore: remove deprecated fee, vesting module's proto files
@tkkwon1998
Copy link
Member

LGTM

@tkkwon1998 tkkwon1998 merged commit 7ebb314 into Canto-Network:main Aug 6, 2023
8 checks passed
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