Skip to content
This repository was archived by the owner on Jan 20, 2026. It is now read-only.

Fixing Banksend unit tests #77

Merged
BrandonWeng merged 2 commits into from
Oct 28, 2022
Merged

Fixing Banksend unit tests #77

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50a2c96
Fix Unit test
BrandonWeng Oct 28, 2022
5c3941e
Fix Unit test
BrandonWeng Oct 28, 2022
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
38 changes: 29 additions & 9 deletions x/bank/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,19 @@ func (k BaseKeeper) DeferredSendCoinsFromModuleToAccount(
return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", recipientAddr)
}

// Branch Context for validation and fail if the module doesn't have enough coins
// but don't write this to the underlying store
validationContext, _ := ctx.CacheContext()
err := k.subUnlockedCoins(validationContext, moduleAddr, amount)
if err != nil {
return err
// Subtract from any pending sends fist
ok := ctx.ContextMemCache().SafeSubDeferredSends(moduleAccount, amount)
if !ok {
// Branch Context for validation and fail if the module doesn't have enough coins
// but don't write this to the underlying store
validationContext, _ := ctx.CacheContext()
err := k.subUnlockedCoins(validationContext, moduleAddr, amount)
if err != nil {
return err
}
}

err = k.addCoins(ctx, recipientAddr, amount)
err := k.addCoins(ctx, recipientAddr, amount)
if err != nil {
return err
}
Expand Down Expand Up @@ -619,6 +623,14 @@ func (k BaseKeeper) destroyCoins(ctx sdk.Context, moduleName string, amounts sdk
// It will panic if the module account does not exist or is unauthorized.
func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amounts sdk.Coins) error {
subFn := func(ctx sdk.Context, moduleName string, amounts sdk.Coins) error {

// Try subtract from the in mem var first, prevents the condition where
// the module may have a pending deposit that would be enough to pay for this send
ok := ctx.ContextMemCache().SafeSubDeferredSends(moduleName, amounts)
Copy link
Contributor

@udpatil udpatil Oct 28, 2022

Choose a reason for hiding this comment

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

is it safe to add this in the normal BurnCoins and not the DeferredBurn? Doesn't that make the deferred mem resources a potential race condition?

Copy link
Contributor Author

@BrandonWeng BrandonWeng Oct 28, 2022

Choose a reason for hiding this comment

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

This is in the case where we call deferredMint and then call the regular burn, in this case if we don't check the deferred sends then it will error out with insufficient balance.

The deferred in mem variables are all blocked by mutex, I think in this case we only care about the absolute value of the in-mem variables at the end of FinalizeBlocker, as only a single value is written to the store

Copy link
Contributor

@udpatil udpatil Oct 28, 2022

Choose a reason for hiding this comment

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

gotcha, makes sense

if ok {
return nil
}

acc := k.ak.GetModuleAccount(ctx, moduleName)
return k.subUnlockedCoins(ctx, acc.GetAddress(), amounts)
}
Expand All @@ -642,8 +654,16 @@ func (k BaseKeeper) DeferredBurnCoins(ctx sdk.Context, moduleName string, amount
// Branch Context for validation and fail if the module doesn't have enough coins
// but don't write this to the underlying store
validationContext, _ := ctx.CacheContext()
acc := k.ak.GetModuleAccount(ctx, moduleName)
err := k.subUnlockedCoins(validationContext, acc.GetAddress(), amounts)
moduleAcc := k.ak.GetModuleAccount(ctx, moduleName)

// Try subtract from the in mem var first, prevents the condition where
// the module may have a pending deposit that would be enough to pay for this send
ok := ctx.ContextMemCache().SafeSubDeferredSends(moduleName, amounts)
if ok {
return nil
}

err := k.subUnlockedCoins(validationContext, moduleAcc.GetAddress(), amounts)
if err != nil {
return err
}
Expand Down