Skip to content

Commit

Permalink
Remove escrow account in wei logic (#434)
Browse files Browse the repository at this point in the history 
## Describe your changes and provide context
Since add and sub balance are now decoupled, we no longer need an
explicit escrow account, and can implicitly represent "escrow" by direct
crediting/debiting account's usei balances. This PR removes the explicit
escrow account logic, and also added wei logic in invariant checks

## Testing performed to validate your change
unit tests
  • Loading branch information
@codchen @udpatil
codchen authored and udpatil committed Mar 26, 2024
1 parent 5bd063f commit c02ec12
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 70 deletions.
8 changes: 8 additions & 0 deletions types/denom.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ func GetBaseDenom() (string, error) {
return baseDenom, nil
}

func MustGetBaseDenom() string {
bd, err := GetBaseDenom()
if err != nil {
panic(err)
}
return bd
}

// ConvertCoin attempts to convert a coin to a given denomination. If the given
// denomination is invalid or if neither denomination is registered, an error
// is returned.
Expand Down
26 changes: 26 additions & 0 deletions x/bank/keeper/invariants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ func NonnegativeBalanceInvariant(k ViewKeeper) sdk.Invariant {

return false
})
k.IterateAllWeiBalances(ctx, func(addr sdk.AccAddress, balance sdk.Int) bool {
if balance.IsNegative() {
count++
msg += fmt.Sprintf("\t%s has a negative wei balance of %s\n", addr, balance)
}

return false
})

broken := count != 0

Expand All @@ -51,6 +59,7 @@ func NonnegativeBalanceInvariant(k ViewKeeper) sdk.Invariant {
func TotalSupply(k Keeper) sdk.Invariant {
return func(ctx sdk.Context) (string, bool) {
expectedTotal := sdk.Coins{}
weiTotal := sdk.NewInt(0)
supply, _, err := k.GetPaginatedTotalSupply(ctx, &query.PageRequest{Limit: query.MaxLimit})

if err != nil {
Expand All @@ -67,6 +76,23 @@ func TotalSupply(k Keeper) sdk.Invariant {
expectedTotal = expectedTotal.Add(coin)
return false
})
k.IterateAllWeiBalances(ctx, func(addr sdk.AccAddress, balance sdk.Int) bool {
weiTotal = weiTotal.Add(balance)
return false
})
weiInUsei, weiRemainder := SplitUseiWeiAmount(weiTotal)
if !weiRemainder.IsZero() {
return sdk.FormatInvariant(types.ModuleName, "total supply",
fmt.Sprintf(
"\twei remainder: %v\n",
weiRemainder)), true
}
baseDenom, err := sdk.GetBaseDenom()
if err == nil {
expectedTotal = expectedTotal.Add(sdk.NewCoin(baseDenom, weiInUsei))
} else if !weiInUsei.IsZero() {
return sdk.FormatInvariant(types.ModuleName, "total supply", "non-zero wei balance without base denom"), true
}

broken := !expectedTotal.IsEqual(supply)

Expand Down
21 changes: 7 additions & 14 deletions x/bank/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ func (suite *IntegrationTestSuite) initKeepersWithmAccPerms(blockedAddrs map[str
appCodec := simapp.MakeTestEncodingConfig().Marshaler

maccPerms[holder] = nil
maccPerms[types.WeiEscrowName] = nil
maccPerms[authtypes.Burner] = []string{authtypes.Burner}
maccPerms[authtypes.Minter] = []string{authtypes.Minter}
maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking}
Expand Down Expand Up @@ -112,50 +111,44 @@ func (suite *IntegrationTestSuite) SetupTest() {
func (suite *IntegrationTestSuite) TestSendCoinsAndWei() {
ctx := suite.ctx
require := suite.Require()
authKeeper, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool))
sdk.RegisterDenom(sdk.DefaultBondDenom, sdk.OneDec())
_, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool))
amt := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)))
require.NoError(keeper.MintCoins(ctx, authtypes.Minter, amt))
addr1 := sdk.AccAddress([]byte("addr1_______________"))
addr2 := sdk.AccAddress([]byte("addr2_______________"))
addr3 := sdk.AccAddress([]byte("addr3_______________"))
require.NoError(keeper.SendCoinsFromModuleToAccount(ctx, authtypes.Minter, addr1, amt))
// should no-op if sending zero
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr2, nil, sdk.DefaultBondDenom, sdk.ZeroInt(), sdk.ZeroInt()))
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr2, sdk.ZeroInt(), sdk.ZeroInt()))
require.Equal(sdk.ZeroInt(), keeper.GetWeiBalance(ctx, addr1))
require.Equal(sdk.ZeroInt(), keeper.GetWeiBalance(ctx, addr2))
require.Equal(sdk.NewInt(100), keeper.GetBalance(ctx, addr1, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.ZeroInt(), keeper.GetBalance(ctx, addr2, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.ZeroInt(), keeper.GetBalance(ctx, authKeeper.GetModuleAddress(types.WeiEscrowName), sdk.DefaultBondDenom).Amount)
// should just do usei send if wei is zero
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, nil, sdk.DefaultBondDenom, sdk.NewInt(50), sdk.ZeroInt()))
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, sdk.NewInt(50), sdk.ZeroInt()))
require.Equal(sdk.ZeroInt(), keeper.GetWeiBalance(ctx, addr1))
require.Equal(sdk.ZeroInt(), keeper.GetWeiBalance(ctx, addr3))
require.Equal(sdk.NewInt(50), keeper.GetBalance(ctx, addr1, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.NewInt(50), keeper.GetBalance(ctx, addr3, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.ZeroInt(), keeper.GetBalance(ctx, authKeeper.GetModuleAddress(types.WeiEscrowName), sdk.DefaultBondDenom).Amount)
// should return error if wei amount overflows
require.Error(keeper.SendCoinsAndWei(ctx, addr1, addr2, nil, sdk.DefaultBondDenom, sdk.ZeroInt(), sdk.NewInt(1_000_000_000_000)))
// sender gets escrowed one usei, recipient does not get redeemed
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr2, nil, sdk.DefaultBondDenom, sdk.NewInt(1), sdk.NewInt(1)))
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr2, sdk.NewInt(1), sdk.NewInt(1)))
require.Equal(sdk.NewInt(999_999_999_999), keeper.GetWeiBalance(ctx, addr1))
require.Equal(sdk.OneInt(), keeper.GetWeiBalance(ctx, addr2))
require.Equal(sdk.NewInt(48), keeper.GetBalance(ctx, addr1, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.OneInt(), keeper.GetBalance(ctx, addr2, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.OneInt(), keeper.GetBalance(ctx, authKeeper.GetModuleAddress(types.WeiEscrowName), sdk.DefaultBondDenom).Amount)
// sender does not get escrowed due to sufficient wei balance, recipient does not get redeemed
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, nil, sdk.DefaultBondDenom, sdk.NewInt(1), sdk.NewInt(999_999_999_999)))
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, sdk.NewInt(1), sdk.NewInt(999_999_999_999)))
require.Equal(sdk.ZeroInt(), keeper.GetWeiBalance(ctx, addr1))
require.Equal(sdk.NewInt(999_999_999_999), keeper.GetWeiBalance(ctx, addr3))
require.Equal(sdk.NewInt(47), keeper.GetBalance(ctx, addr1, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.NewInt(51), keeper.GetBalance(ctx, addr3, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.OneInt(), keeper.GetBalance(ctx, authKeeper.GetModuleAddress(types.WeiEscrowName), sdk.DefaultBondDenom).Amount)
// sender gets escrowed and recipient gets redeemed
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, nil, sdk.DefaultBondDenom, sdk.NewInt(1), sdk.NewInt(2)))
require.NoError(keeper.SendCoinsAndWei(ctx, addr1, addr3, sdk.NewInt(1), sdk.NewInt(2)))
require.Equal(sdk.NewInt(999_999_999_998), keeper.GetWeiBalance(ctx, addr1))
require.Equal(sdk.NewInt(1), keeper.GetWeiBalance(ctx, addr3))
require.Equal(sdk.NewInt(45), keeper.GetBalance(ctx, addr1, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.NewInt(53), keeper.GetBalance(ctx, addr3, sdk.DefaultBondDenom).Amount)
require.Equal(sdk.OneInt(), keeper.GetBalance(ctx, authKeeper.GetModuleAddress(types.WeiEscrowName), sdk.DefaultBondDenom).Amount)
}

func (suite *IntegrationTestSuite) TestSupply() {
Expand Down
90 changes: 36 additions & 54 deletions x/bank/keeper/send.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"errors"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
"github.com/cosmos/cosmos-sdk/telemetry"
Expand All @@ -20,11 +18,11 @@ type SendKeeper interface {
InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error
SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsWithoutAccCreation(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int, wei sdk.Int) error
SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to sdk.AccAddress, amt sdk.Int, wei sdk.Int) error
SubUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, checkNeg bool) error
AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, checkNeg bool) error
SubWei(ctx sdk.Context, addr sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int) error
AddWei(ctx sdk.Context, addr sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int) error
SubWei(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Int) error
AddWei(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Int) error

GetParams(ctx sdk.Context) types.Params
SetParams(ctx sdk.Context, params types.Params)
Expand All @@ -36,7 +34,7 @@ type SendKeeper interface {
}

var _ SendKeeper = (*BaseSendKeeper)(nil)
var MaxWeiBalance sdk.Int = sdk.NewInt(1_000_000_000_000)
var OneUseiInWei sdk.Int = sdk.NewInt(1_000_000_000_000)

// BaseSendKeeper only allows transfers between accounts without the possibility of
// creating coins. It implements the SendKeeper interface.
Expand Down Expand Up @@ -332,75 +330,59 @@ func (k BaseSendKeeper) BlockedAddr(addr sdk.AccAddress) bool {
return k.blockedAddrs[addr.String()]
}

func (k BaseSendKeeper) SubWei(ctx sdk.Context, addr sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int) error {
func (k BaseSendKeeper) SubWei(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Int) error {
if amt.Equal(sdk.ZeroInt()) {
return nil
}
if amt.GTE(MaxWeiBalance) {
return errors.New("cannot send more than 10^12 wei")
currentWeiBalance := k.GetWeiBalance(ctx, addr)
if amt.LTE(currentWeiBalance) {
// no need to change usei balance
return k.setWeiBalance(ctx, addr, currentWeiBalance.Sub(amt))
}
escrow := customEscrow
if escrow == nil {
escrow = k.ak.GetModuleAddress(types.WeiEscrowName)
currentUseiBalance := k.GetBalance(ctx, addr, sdk.MustGetBaseDenom()).Amount
currentAggregatedBalance := currentUseiBalance.Mul(OneUseiInWei).Add(currentWeiBalance)
postAggregatedbalance := currentAggregatedBalance.Sub(amt)
if postAggregatedbalance.IsNegative() {
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%swei is smaller than %swei", currentAggregatedBalance, amt)
}
currentWeiBalance := k.GetWeiBalance(ctx, addr)
postWeiBalance := currentWeiBalance.Sub(amt)
if postWeiBalance.GTE(sdk.ZeroInt()) {
if err := k.setWeiBalance(ctx, addr, postWeiBalance); err != nil {
return err
}
} else {
if err := k.setWeiBalance(ctx, addr, MaxWeiBalance.Add(postWeiBalance)); err != nil {
// postWeiBalanceFrom is negative
return err
}
// need to send one sei to escrow because wei balance is insufficient
if err := k.sendCoinsWithoutAccCreation(ctx, addr, escrow, sdk.NewCoins(sdk.NewCoin(denom, sdk.OneInt())), false); err != nil {
return err
}
useiBalance, weiBalance := SplitUseiWeiAmount(postAggregatedbalance)
if err := k.setBalance(ctx, addr, sdk.NewCoin(sdk.MustGetBaseDenom(), useiBalance), true); err != nil {
return err
}
return nil
return k.setWeiBalance(ctx, addr, weiBalance)
}

func (k BaseSendKeeper) AddWei(ctx sdk.Context, addr sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int) error {
func (k BaseSendKeeper) AddWei(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Int) error {
if amt.Equal(sdk.ZeroInt()) {
return nil
}
if amt.GTE(MaxWeiBalance) {
return errors.New("cannot send more than 10^12 wei")
}
escrow := customEscrow
if escrow == nil {
escrow = k.ak.GetModuleAddress(types.WeiEscrowName)
}
currentWeiBalance := k.GetWeiBalance(ctx, addr)
postWeiBalance := currentWeiBalance.Add(amt)
if postWeiBalance.LT(MaxWeiBalance) {
if err := k.setWeiBalance(ctx, addr, postWeiBalance); err != nil {
return err
}
} else {
if err := k.setWeiBalance(ctx, addr, postWeiBalance.Sub(MaxWeiBalance)); err != nil {
return err
}
// need to redeem one sei from escrow because wei balance overflowed
one := sdk.NewCoins(sdk.NewCoin(denom, sdk.OneInt()))
if err := k.sendCoinsWithoutAccCreation(ctx, escrow, addr, one, false); err != nil {
return err
}
if postWeiBalance.LT(OneUseiInWei) {
// no need to change usei balance
return k.setWeiBalance(ctx, addr, postWeiBalance)
}
return nil
currentUseiBalance := k.GetBalance(ctx, addr, sdk.MustGetBaseDenom()).Amount
useiCredit, weiBalance := SplitUseiWeiAmount(postWeiBalance)
if err := k.setBalance(ctx, addr, sdk.NewCoin(sdk.MustGetBaseDenom(), currentUseiBalance.Add(useiCredit)), true); err != nil {
return err
}
return k.setWeiBalance(ctx, addr, weiBalance)
}

func (k BaseSendKeeper) SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to sdk.AccAddress, customEscrow sdk.AccAddress, denom string, amt sdk.Int, wei sdk.Int) error {
if err := k.SubWei(ctx, from, customEscrow, denom, wei); err != nil {
func (k BaseSendKeeper) SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to sdk.AccAddress, amt sdk.Int, wei sdk.Int) error {
if err := k.SubWei(ctx, from, wei); err != nil {
return err
}
if err := k.AddWei(ctx, to, customEscrow, denom, wei); err != nil {
if err := k.AddWei(ctx, to, wei); err != nil {
return err
}
if amt.GT(sdk.ZeroInt()) {
return k.SendCoinsWithoutAccCreation(ctx, from, to, sdk.NewCoins(sdk.NewCoin(denom, amt)))
return k.SendCoinsWithoutAccCreation(ctx, from, to, sdk.NewCoins(sdk.NewCoin(sdk.MustGetBaseDenom(), amt)))
}
return nil
}

func SplitUseiWeiAmount(amt sdk.Int) (sdk.Int, sdk.Int) {
return amt.Quo(OneUseiInWei), amt.Mod(OneUseiInWei)
}
19 changes: 19 additions & 0 deletions x/bank/keeper/view.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type ViewKeeper interface {

IterateAccountBalances(ctx sdk.Context, addr sdk.AccAddress, cb func(coin sdk.Coin) (stop bool))
IterateAllBalances(ctx sdk.Context, cb func(address sdk.AccAddress, coin sdk.Coin) (stop bool))
IterateAllWeiBalances(ctx sdk.Context, cb func(sdk.AccAddress, sdk.Int) bool)
}

// BaseViewKeeper implements a read only keeper implementation of ViewKeeper.
Expand Down Expand Up @@ -247,3 +248,21 @@ func (k BaseViewKeeper) GetWeiBalance(ctx sdk.Context, addr sdk.AccAddress) sdk.
}
return *res
}

func (k BaseViewKeeper) IterateAllWeiBalances(ctx sdk.Context, cb func(sdk.AccAddress, sdk.Int) bool) {
store := prefix.NewStore(ctx.KVStore(k.storeKey), types.WeiBalancesPrefix)

iterator := store.Iterator(nil, nil)
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
val := new(sdk.Int)
if err := val.Unmarshal(iterator.Value()); err != nil {
// should never happen
panic(err)
}
if cb(iterator.Key(), *val) {
break
}
}
}
2 changes: 0 additions & 2 deletions x/bank/types/key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ const (

// QuerierRoute defines the module's query routing key
QuerierRoute = ModuleName

WeiEscrowName = "weiescrow"
)

// KVStore keys
Expand Down

0 comments on commit c02ec12

Please sign in to comment.