Removal of the Liquidity Module on Gaia

[dongsam]

Problem, Background

To remove the liquidity module from Gaia, we need to discussion of what to do with the remaining pool coins of holders and the reserved coins. Two options have been proposed: force withdrawal and sending the coins to the community fund.

Problem details

Options for Dealing with Remaining Pool Coins and Reserved Coins:

Force Withdrawal:

• first step : forcefully withdraw all pool coins of holders that exist in the Cosmos Hub except for ibc escrowed coins. This would be done during the first upgrade, which could be Gaia v10.
• second step : completely remove the liquidity module during the second upgrade, which could be Gaia v11.
• pros : remaining pool coins can be withdrawn/burned as much as possible.
• cons : forced withdrawal is an artificial action that changes the account balance and requires extensive testing and simulations.
• For accounts that may be module accounts because they don't have Pubkey like the IBC escrow acc, pool coins should not be forcefully withdrawn as this may cause problems when the pool coins return via IBC.

Send to the Community Fund:

• send all reserved coins to the community fund, This would be done during the first upgrade, which could be Gaia v10.
• pros : the process is simple and development can proceed immediately.
• cons : pool coin holders will suffer an economic loss and several non-atom coins will be placed in community funds.

Context

The decision was made to proceed with the forced withdrawal option because the total reserved coin value is not small, Sending the coins to the community fund would be easier if the remaining reserve value in the module was low, but It's more than about $100,000 (the balance of the reserve_account_addresses in /cosmos/liquidity/v1beta1/pools), and the remaining reserved coin value due to IBC escrow and module accounts is about $1000.

Decision

The following decisions need to be made:

1. Assuming that the final confirmation has been made through a forced withdrawal method, whether to start development or to proceed with a signal proposal first to determine the direction and start development.
2. If force withdrawal is chosen, what should be done with the remaining value of module accounts? Should it be sent to the community fund?
3. The SDK version to be used. If the version of using the SDK in v10 Gaia is expected, we can start development based on the version. Otherwise, we can start with based v0.45.x first.

Closing criteria

• all decisions have been made
• Liquidity module removal PR merged

[dongsam]

The development of force withdrawal function based on cosmos-sdk v0.45.x is underway in Gravity-Devs/liquidity#30 PR. First of all, we put the logic in the module's migration, but if necessary, we can move it to Gaia's upgrade handler

Simulation

liquidity module force withdrawal simulation result (cosmos-hub state height 14922680 based)

• addresses_having_poolcoins_[before]
• addresses_having_poolcoins_[after]
• pools_state_[before]
• pools_state_[after]

Decision

• It is necessary to decide how to handle the reserve coins for the pool coins remaining in the 10 module accounts after the forced withdrawal, whether to send them to the community fund or leave them at the reserve addresses

[mmulji-ic]

Draft signalling prop https://docs.google.com/document/d/1NGoKb4ebKbhW2-ttFWMcdCLjKa3fn7SwCbNV8WibxE0/edit#

[mpoke]

@dongsam What's the latest status on Gravity-Devs/liquidity#30? Is there something we can do on our side to help?

[dongsam]

@mpoke Hi, The basic development, testing, and simulation have been completed, and when a decision is made on the below decision, we will reflect the logic and release it, Thanks

• It is necessary to decide how to handle the reserve coins for the pool coins remaining in the 10 module accounts after the forced withdrawal, whether to send them to the community fund or leave them at the reserve addresses

[faddat]

Hello, I do hate to be the bearer of bad news, but I see no realistic path to achieving path unwinding before the end of this year at the earliest.

Other than that when I read this stuff, all I see, in big bold letters, is:

omg dead module lets do Juno prop 16 on the cosmos hub!!!

Except of course that we don't really know who they are, nobody is in contact with them, and they have every reason to believe that whatever they put into the pool stays there.

While I am supportive of removing the liquidity module, I am not supportive of getting into the sort of hellscape that was Juno prop 16, nor am I supportive of encouraging counterparty chains to do mandatory upgrades to ensure that unwinding can work. Therefore, I have created a pull request that reduces the maintenance burden of the liquidity module.

Please note that the pull request in no way claims to remove it, the only claim about that phone request is that it will reduce the burden on Bharvest and the cosmos hub team. It will also make it easier to potentially put the module into some kind of dormant state where logic is removed and functionality is reduced to some sort of withdraw Only thing, if that's even possible.

If I'm missing something, and there is an achievable plan, emphasis on achievable, for removing the liquidity module without Juno proposal 16 on the cosmos hub, I'm all ears.

The message we would be sending

So another thing that just occurred to me on my walk home is that the people who tried the liquidity module are early adopters, and what we would be telling them is that hey on the cosmos hub, we're okay with you getting hit by a steamroller. No big deal.

Can we please come up with any other idea, maybe like the one I implemented in the PR?

[jtremback]

As I understand this:

• There is around ~$100,000 still in various pools.
• The migration will automatically move 99% of these remaining balances in the pools back into the addresses of their owners.
• 1% of the remaining balances (~$1000?) are not possible to automatically move back into the addresses of their owners. I don't completely understand, but this is because the addresses holding them are from the IBC transfer module and we can't figure out who owns them on the other chain?
• This remaining 1% is what is proposed to be moved to the community pool, for manual transfer out to their owners once this can be figured out.

@dongsam is this correct?

If this is true, then it seems like a huge waste of time to maintain a whole deprecated module for $1000 in unwithdrawn balances from a handful of accounts. If we move the coins to the community pool, then it won't even be lost, and can be sent out to their owners once we figure that out.

[dongsam]

@jtremback Yes correct.

• Withdraw is performed for all accounts whose ownership of the account is confirmed due to the presence of pubkey or sequence
• However, for 10 accounts (which can be checked addresses_having_poolcoins_[after]) that may be used as module accounts because there is no sequence or pubkey, forced withdrawal is not carried out because the escrow may break and unexpected results may occur in other modules such as IBC
• As a result, you can check the amount of coins left in the pool reserve without being forcibly withdrawn pools_state_[after]

[faddat]

How about the GAMM equivalent that has been scattered across cosmos?

[jtremback]

How about the GAMM equivalent that has been scattered across cosmos?

@dongsam Yes good question. I assume that these are represented by the 1% which can not be force withdrawn, right? Those accounts are IBC transfer escrow accounts?

[jtremback]

@dongsam I've added the section below to the proposal- can you check it for correctness?

Currently, the Liquidity module contains around ~$100,000 worth of un-withdrawn tokens. The developers of the Liquidity module have kindly prepared some code to facilitate the removal of this module and get any remaining funds back to their rightful owners. Here is how this will happen:

• During the next planned Hub upgrade, a migration script will run which will return ~99% of the unwithdrawn tokens directly back to their owners.
• ~1% of tokens (worth ~$1000) will remain which cannot be attributed to an owner. This is because they are owned by module accounts.
• This remaining ~1% will be transferred to the community pool. If in the future, the rightful owners come forward with evidence of their ownership, these tokens can be returned to them by governance proposal.

[dongsam]

@jtremback

@dongsam Yes good question. I assume that these are represented by the 1% which can not be force withdrawn, right? Those accounts are IBC transfer escrow accounts?

That's right. More accurately, there are 10 accounts that hold 1% that cannot be withdrawn, 5 are ibc esrow accounts, and the other 5 are addresses that are not for ibc, but may be used as module accounts because pubkey or sequence does not exist (These accounts may not be 0% likely to be owned by individuals, but cannot prove ownership within the chain )

@dongsam I've added the section below to the proposal- can you check it for correctness?

Sure, I checked that there is no problem without "This is because they are owned by module accounts.", (Please refer to the above answer)

Anyway It seems that the proposal direction has been decided, I will update Gravity-Devs/liquidity#30 PR code to move the remaining 1% that cannot be withdrawn to the community fund as described in the proposal

[jtremback]

Thanks @dongsam !

[dongsam]

Gravity-Devs/liquidity#30 is ready for review now, Please review and comment when you have time, and I will release it so that Gaia can import it after merged

[dongsam]

Hi Gaia team, Gravity-Devs/liquidity#30 this PR just reviewed and applied the suggestions, and what is the next step? do we need additional review or should I release the new liquidity version with the PR merge?

Also, I expected this work to be included in gaia v10, but it seems that v10 has already been released and will be upgraded, I think additional modifications may be needed depending on the cosmos-sdk version that would be used in v11 or targeted Gaia version

[mmulji-ic]

Proposal on-chain: https://www.mintscan.io/cosmos/proposals/801

[faddat]

Oh super I will head out and vote yes

…

On Wed, Jun 14, 2023, 9:26 PM Milan Mulji ***@***.***> wrote: Proposal on-chain: https://www.mintscan.io/cosmos/proposals/801 — Reply to this email directly, view it on GitHub <#2346 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABWPVCKFN4AAGEH7TNJNUALXLG3WZANCNFSM6AAAAAAWM2AEB4> . You are receiving this because you commented.Message ID: ***@***.***>

[mpoke]

Also, I expected this work to be included in gaia v10, but it seems that v10 has already been released and will be upgraded, I think additional modifications may be needed depending on the cosmos-sdk version that would be used in v11 or targeted Gaia version

@dongsam Indeed, the work will end up in Gaia v11. See #2407 for more details. v11 will also use SDK 0.45, so I don't think any changes are needed.

[mpoke]

@dongsam it seems that https://www.mintscan.io/cosmos/proposals/801 is going to pass. What's the latest on getting a Liquidity release with the migration code that we can import into Gaia? Also, do you have suggestions of things we should check in the testnet before adding these changes to mainnet?

[dongsam]

Hi @mpoke I just released liquidity v1.6.0-forced-withdrawal-rc1 version based on Gravity-Devs/liquidity#30 branch

It was released as RC first because additional modifications could occur depending on additional reviews or the dependency of the Gaia in which the version will be used

The ConsensusVersion of the module has increased from 2 to 3, so the migration could be automatically performed from a UpgradeHandler through RunMigrations

The official testnet may not have liquidity module's states to conduct meaningful tests, so it seems necessary to create a temporary testnet based on the latest cosmos-hub mainnet state, perform upgrades with migration tests there, and verify the invariant

[mpoke]

Great. Thanks @dongsam. We'll integrate the changes to Gaia. Please note that there is a typo in the name of the release as compared to the name of the tag: v1.6.0-force-withdrawal-rc1 vs v1.6.0-forced-withdrawal-rc1

[mpoke]

@dongsam could you please cut a final release for v1.6.0-forced-withdrawal?

[dongsam]

@mpoke Sure, I just release v1.6.0-forced-withdrawal

here is changelog from rc1 Gravity-Devs/liquidity@v1.6.0-forced-withdrawal-rc1...v1.6.0-forced-withdrawal
It includes only bump cometBFT to v0.34.29

[mpoke]

@dongsam Thanks.

Just a nit: the following line should be removed from the release notes

This is a release candidate version, additional changes may be occurred depending on additional reviews or the dependency of the Gaia in which the version will be used