# Liquidity Module Removal PR review
[PR #30](https://github.com/Gravity-Devs/liquidity/pull/30)

## Conclusion

No issues were found in the review. Some stuff can be simplified, and some details could benefit from further clarification.

### [x/liquidity/module.go](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/module.go#L130)

The change on line [130](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/module.go#L130), which caues the`RegisterInvariants` func to perfom a no-op, lacks documentation and could be clarified.

### [x/liquidity/keeper/migration_test.go](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration_test.go)

In the `TestMigration` test, an assertion should be added to verify that the pool's reserve account is emptied after executing `SafeForceWithdrawal()` without errors. The same should be done in the `TestMigrationFailCase` test, which doesn't check that the pool's `reserve_account` and the community fund balances remain unchanged after `SafeForceWithdrawal()` panics on line [188](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration_test.go#L188)].

### [/x/liquidity/keeper/migration.go](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration.go)

Nit: in line [77](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration.go#L77) and [106](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration.go#106), the condition to perform user pool coin withdrawal can be refactored from:
`acc.GetSequence() != 0 || acc.GetPubKey() != nil` to `acc.GetPubKey() != nil` since account sequences can't be incremented without a signing key.

Nit: The [`SafeForceWithdrawal`](https://github.com/Gravity-Devs/liquidity/blob/7bfed3b7c91696e5b546e3a678765ff3503e9f75/x/liquidity/keeper/migration.go#37) func has an obsolete `broken` local variable that's is always set to false within the function scope. The func can be refactored as follows:

func SafeForceWithdrawal(ctx sdk.Context, keeper Keeper, bankKeeper liquiditytypes.BankKeeper, accountKeeper liquiditytypes.AccountKeeper) error {
	defer func() {
		recover()
	}()

	cachedCtx, writeCache := ctx.CacheContext()
	err := ForceWithdrawal(cachedCtx, keeper, bankKeeper, accountKeeper)
	if err == nil {
		writeCache()
	}
	return nil
}