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

transfer (total escrow): add invariant for total escrow #3506

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions modules/apps/transfer/keeper/invariants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
)

// RegisterInvariants registers all transfer invariants
func RegisterInvariants(ir sdk.InvariantRegistry, k *Keeper) {
ir.RegisterRoute(types.ModuleName, "total-escrow-per-denom",
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this string is arbitrary and doesn't hold too much significance?

Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't seem to be treated as very significant in the sdk equivalent impl.

TotalEscrowPerDenomInvariants(k))
}

// AllInvariants runs all invariants of the transfer module.
func AllInvariants(k *Keeper) sdk.Invariant {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should add a call to this in the tests?

return func(ctx sdk.Context) (string, bool) {
return TotalEscrowPerDenomInvariants(k)(ctx)
}
}

// TotalEscrowPerDenomInvariants checks that the total amount escrowed for
// each denom is not smaller than the amount stored in the state entry.
func TotalEscrowPerDenomInvariants(k *Keeper) sdk.Invariant {
return func(ctx sdk.Context) (string, bool) {
var (
msg string
broken bool
totalEscrowedInAccounts sdk.Coins
)

totalEscrowedInState := k.GetAllTotalEscrowed(ctx)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

portID := k.GetPort(ctx)
transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, portID)
for _, channel := range transferChannels {
escrowAddress := types.GetEscrowAddress(portID, channel.ChannelId)
escrowBalances := k.bankKeeper.GetAllBalances(ctx, escrowAddress)

totalEscrowedInAccounts = totalEscrowedInAccounts.Add(escrowBalances...)
}

for _, expectedEscrow := range totalEscrowedInState {
if found, actualEscrow := totalEscrowedInAccounts.Find(expectedEscrow.GetDenom()); found {
broken = expectedEscrow.Amount.GT(actualEscrow.Amount)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
msg += fmt.Sprintf("\tdenom: %s, actual escrow (%s) is < expected escrow (%s)\n", expectedEscrow.GetDenom(), actualEscrow.Amount, expectedEscrow.Amount)
}
}

if broken {
// the total amount for each denom in escrow should be >= the amount stored in state for each denom
return sdk.FormatInvariant(
types.ModuleName,
"total escrow per denom invariance",
fmt.Sprintf("found denom(s) with total escrow amount lower than expected:\n%s", msg)), broken
}

return "", broken
DimitrisJim marked this conversation as resolved.
Show resolved Hide resolved
}
}
77 changes: 77 additions & 0 deletions modules/apps/transfer/keeper/invariants_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package keeper_test

import (
"fmt"

"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
)

func (suite *KeeperTestSuite) TestTotalEscrowPerDenomInvariant() {
testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success",
func() {},
true,
},
{
"fails with broken invariant",
func() {
// set amount for denom higher than actual value in escrow
amount := math.NewInt(200)
suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.DefaultBondDenom, amount)
},
false,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
path := NewTransferPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)
ctx := suite.chainA.GetContext()

amount := math.NewInt(100)

// send coins from chain A to chain B so that we have them in escrow
coin := sdk.NewCoin(sdk.DefaultBondDenom, amount)
msg := types.NewMsgTransfer(
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ChannelID,
coin,
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
suite.chainA.GetTimeoutHeight(), 0, "",
)

res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg)
suite.Require().NoError(err)
suite.Require().NotNil(res)

tc.malleate()

out, broken := keeper.TotalEscrowPerDenomInvariants(&suite.chainA.GetSimApp().TransferKeeper)(ctx)

if tc.expPass {
suite.Require().False(broken)
suite.Require().Empty(out)
} else {
suite.Require().True(broken)

escrow := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(ctx, sdk.DefaultBondDenom)
expOut := fmt.Sprintf("denom: %s, actual escrow (%s) is < expected escrow (%s)\n", sdk.DefaultBondDenom, amount, escrow)
suite.Require().Contains(out, expOut)
}
})
}
}
4 changes: 2 additions & 2 deletions modules/apps/transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ func NewAppModule(k keeper.Keeper) AppModule {
}

// RegisterInvariants implements the AppModule interface
func (AppModule) RegisterInvariants(ir sdk.InvariantRegistry) {
// TODO
func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) {
keeper.RegisterInvariants(ir, &am.keeper)
}

// RegisterServices registers module services.
Expand Down