Skip to content

Commit e850381

Browse files
troiantroian
committed
refactor(x/escrow): truncate dec part during payment withdraw
fixes issue when fractional of uakt remain in the balance after account is closed. refactor escrow testsuite to ensure funds are tracked during account operations and match expected values Signed-off-by: Artur Troian <[email protected]>
1 parent 9e1d6cf commit e850381

File tree

9 files changed

+457
-30
lines changed

9 files changed

+457
-30
lines changed

‎testutil/state/suite.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,8 @@ func SetupTestSuiteWithKeepers(t testing.TB, keepers Keepers) *TestSuite {
7676

7777
if keepers.Bank == nil {
7878
bkeeper := &emocks.BankKeeper{}
79-
bkeeper.
80-
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
81-
Return(nil)
82-
bkeeper.
83-
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
84-
Return(nil)
85-
bkeeper.
86-
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
87-
Return(nil)
79+
// do not set bank mock during suite setup, each test must set them manually
80+
// to make sure escrow balance values are tracked correctly
8881
bkeeper.
8982
On("SpendableCoin", mock.Anything, mock.Anything, mock.Anything).
9083
Return(sdk.NewInt64Coin("uakt", 10000000))
@@ -169,6 +162,10 @@ func SetupTestSuiteWithKeepers(t testing.TB, keepers Keepers) *TestSuite {
169162
}
170163
}
171164

165+
func (ts *TestSuite) PrepareMocks(fn func(ts *TestSuite)) {
166+
fn(ts)
167+
}
168+
172169
func (ts *TestSuite) App() *app.AkashApp {
173170
return ts.app
174171
}

‎x/deployment/keeper/grpc_query_test.go‎

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
sdk "github.com/cosmos/cosmos-sdk/types"
1111
sdkquery "github.com/cosmos/cosmos-sdk/types/query"
1212
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/mock"
1314
"github.com/stretchr/testify/require"
1415
deposit "pkg.akt.dev/go/node/types/deposit/v1"
1516

@@ -25,6 +26,7 @@ import (
2526
)
2627

2728
type grpcTestSuite struct {
29+
*state.TestSuite
2830
t *testing.T
2931
app *app.AkashApp
3032
ctx sdk.Context
@@ -39,6 +41,7 @@ type grpcTestSuite struct {
3941
func setupTest(t *testing.T) *grpcTestSuite {
4042
ssuite := state.SetupTestSuite(t)
4143
suite := &grpcTestSuite{
44+
TestSuite: ssuite,
4245
t: t,
4346
app: ssuite.App(),
4447
ctx: ssuite.Context(),
@@ -60,17 +63,29 @@ func setupTest(t *testing.T) *grpcTestSuite {
6063
func TestGRPCQueryDeployment(t *testing.T) {
6164
suite := setupTest(t)
6265

66+
suite.PrepareMocks(func(ts *state.TestSuite) {
67+
bkeeper := ts.BankKeeper()
68+
69+
bkeeper.
70+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
71+
Return(nil)
72+
bkeeper.
73+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
74+
Return(nil)
75+
bkeeper.
76+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
77+
Return(nil)
78+
})
79+
6380
// creating deployment
6481
deployment, groups := suite.createDeployment()
6582
err := suite.keeper.Create(suite.ctx, deployment, groups)
6683
require.NoError(t, err)
6784

6885
eid := suite.createEscrowAccount(deployment.ID)
6986

70-
var (
71-
req *v1beta4.QueryDeploymentRequest
72-
expDeployment v1beta4.QueryDeploymentResponse
73-
)
87+
var req *v1beta4.QueryDeploymentRequest
88+
var expDeployment v1beta4.QueryDeploymentResponse
7489

7590
testCases := []struct {
7691
msg string
@@ -138,6 +153,19 @@ func TestGRPCQueryDeployment(t *testing.T) {
138153

139154
func TestGRPCQueryDeployments(t *testing.T) {
140155
suite := setupTest(t)
156+
suite.PrepareMocks(func(ts *state.TestSuite) {
157+
bkeeper := ts.BankKeeper()
158+
159+
bkeeper.
160+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
161+
Return(nil)
162+
bkeeper.
163+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
164+
Return(nil)
165+
bkeeper.
166+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
167+
Return(nil)
168+
})
141169

142170
// creating deployments with different states
143171
deployment, groups := suite.createDeployment()
@@ -258,6 +286,19 @@ type deploymentFilterModifier struct {
258286

259287
func TestGRPCQueryDeploymentsWithFilter(t *testing.T) {
260288
suite := setupTest(t)
289+
suite.PrepareMocks(func(ts *state.TestSuite) {
290+
bkeeper := ts.BankKeeper()
291+
292+
bkeeper.
293+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
294+
Return(nil)
295+
bkeeper.
296+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
297+
Return(nil)
298+
bkeeper.
299+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
300+
Return(nil)
301+
})
261302

262303
// creating orders with different states
263304
depA, _ := createActiveDeployment(t, suite.ctx, suite.keeper)
@@ -452,6 +493,20 @@ func TestGRPCQueryDeploymentsWithFilter(t *testing.T) {
452493
func TestGRPCQueryGroup(t *testing.T) {
453494
suite := setupTest(t)
454495

496+
suite.PrepareMocks(func(ts *state.TestSuite) {
497+
bkeeper := ts.BankKeeper()
498+
499+
bkeeper.
500+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
501+
Return(nil)
502+
bkeeper.
503+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
504+
Return(nil)
505+
bkeeper.
506+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
507+
Return(nil)
508+
})
509+
455510
// creating group
456511
deployment, groups := suite.createDeployment()
457512
err := suite.keeper.Create(suite.ctx, deployment, groups)
@@ -525,6 +580,20 @@ func TestGRPCQueryGroup(t *testing.T) {
525580
func (suite *grpcTestSuite) createDeployment() (v1.Deployment, v1beta4.Groups) {
526581
suite.t.Helper()
527582

583+
suite.PrepareMocks(func(ts *state.TestSuite) {
584+
bkeeper := ts.BankKeeper()
585+
586+
bkeeper.
587+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
588+
Return(nil)
589+
bkeeper.
590+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
591+
Return(nil)
592+
bkeeper.
593+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
594+
Return(nil)
595+
})
596+
528597
deployment := testutil.Deployment(suite.t)
529598
group := testutil.DeploymentGroup(suite.t, deployment.ID, 0)
530599
group.GroupSpec.Resources = v1beta4.ResourceUnits{

‎x/escrow/keeper/grpc_query_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
sdkmath "cosmossdk.io/math"
8+
"github.com/stretchr/testify/mock"
89
"github.com/stretchr/testify/require"
910

1011
"github.com/cosmos/cosmos-sdk/baseapp"
@@ -25,6 +26,7 @@ import (
2526
)
2627

2728
type grpcTestSuite struct {
29+
*state.TestSuite
2830
t *testing.T
2931
app *app.AkashApp
3032
ctx sdk.Context
@@ -38,6 +40,8 @@ type grpcTestSuite struct {
3840
func setupTest(t *testing.T) *grpcTestSuite {
3941
ssuite := state.SetupTestSuite(t)
4042
suite := &grpcTestSuite{
43+
TestSuite: ssuite,
44+
4145
t: t,
4246
app: ssuite.App(),
4347
ctx: ssuite.Context(),
@@ -263,6 +267,20 @@ func TestGRPCQueryPayments(t *testing.T) {
263267
}
264268

265269
func (suite *grpcTestSuite) createEscrowAccount(id dv1.DeploymentID) eid.Account {
270+
suite.PrepareMocks(func(ts *state.TestSuite) {
271+
bkeeper := ts.BankKeeper()
272+
273+
bkeeper.
274+
On("SendCoinsFromAccountToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
275+
Return(nil)
276+
bkeeper.
277+
On("SendCoinsFromModuleToAccount", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
278+
Return(nil)
279+
bkeeper.
280+
On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
281+
Return(nil)
282+
})
283+
266284
owner, err := sdk.AccAddressFromBech32(id.Owner)
267285
require.NoError(suite.t, err)
268286

‎x/escrow/keeper/keeper.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ func (k *keeper) AuthorizeDeposits(sctx sdk.Context, msg sdk.Msg) ([]etypes.Depo
243243
return false
244244
}
245245

246-
// Delete is ignored here as not all fund may be used during deployment lifetime.
246+
// Delete is ignored here as not all funds may be used during deployment lifetime.
247247
// also, there can be another deployment using same authorization and may return funds before deposit is fully used
248248
err = k.authzKeeper.SaveGrant(ctx, owner, granter, resp.Updated, expiration)
249249
if err != nil {
@@ -941,8 +941,6 @@ func (acc *account) deductFromBalance(amount sdk.DecCoin) (sdk.DecCoin, bool) {
941941
toWithdraw.AddMut(remaining)
942942
}
943943

944-
funds.Amount.SubMut(toWithdraw)
945-
946944
acc.State.Deposits[i].Balance.Amount.SubMut(toWithdraw)
947945
if acc.State.Deposits[i].Balance.IsZero() {
948946
idx++
@@ -956,16 +954,19 @@ func (acc *account) deductFromBalance(amount sdk.DecCoin) (sdk.DecCoin, bool) {
956954
}
957955
}
958956

957+
// clean empty deposits
959958
if idx > 0 {
960959
acc.State.Deposits = acc.State.Deposits[idx:]
961960
}
962961

962+
funds.Amount.SubMut(withdrew)
963963
res := sdk.NewDecCoinFromDec(amount.Denom, withdrew)
964964

965965
if remaining.IsZero() {
966966
return res, false
967967
}
968968

969+
// at this point the account is overdrawn
969970
funds.Amount.SubMut(remaining)
970971

971972
return res, true
@@ -983,7 +984,7 @@ func accountSettleFullBlocks(acc *account, payments []payment, heightDelta sdkma
983984

984985
for idx := range payments {
985986
p := payments[idx]
986-
paymentTransfer := sdk.NewDecCoinFromDec(p.State.Rate.Denom, p.State.Rate.Amount.Mul(sdkmath.LegacyNewDecFromInt(heightDelta)))
987+
paymentTransfer := sdk.NewDecCoinFromDec(p.State.Rate.Denom, p.State.Rate.Amount.Mul(sdkmath.LegacyNewDecFromInt(heightDelta)).TruncateDec())
987988

988989
paymentsTransfers = append(paymentsTransfers, paymentTransfer)
989990
}
@@ -994,7 +995,7 @@ func accountSettleFullBlocks(acc *account, payments []payment, heightDelta sdkma
994995
unsettledAmount := paymentsTransfers[idx]
995996

996997
settledAmount, od := acc.deductFromBalance(unsettledAmount)
997-
unsettledAmount.Amount.SubMut(unsettledAmount.Amount)
998+
unsettledAmount.Amount.SubMut(settledAmount.Amount)
998999

9991000
if settledAmount.IsPositive() {
10001001
payments[idx].State.Balance.Amount.AddMut(settledAmount.Amount)

‎x/escrow/keeper/keeper_settle_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package keeper
22

33
import (
4+
"fmt"
45
"testing"
56

67
sdkmath "cosmossdk.io/math"
@@ -100,14 +101,42 @@ func TestSettleFullBlocks(t *testing.T) {
100101
},
101102
} {
102103
account, payments, blocks := setupDistTest(t, tt.cfg)
104+
//initialBalance := sdkmath.LegacyMustNewDecFromStr(account.State.Deposits[0].Balance.Amount.String())
103105

104106
overdrawn := accountSettleFullBlocks(&account, payments, blocks)
105107
assert.Equal(t, tt.cfg.balanceEnd < 0, overdrawn, tt.name)
106108

107109
assertAmountEqual(t, sdkmath.LegacyNewDec(tt.cfg.balanceEnd), account.State.Funds[0].Amount, tt.name)
108110

111+
totalTransferred := sdkmath.LegacyZeroDec()
112+
for _, transfer := range tt.cfg.transferred {
113+
totalTransferred.AddMut(transfer)
114+
}
115+
116+
assert.Equal(t, totalTransferred, account.State.Transferred[0].Amount, fmt.Sprintf("%s: deposit balance should be decremented by total transferred", tt.name))
117+
118+
totalPayments := sdkmath.LegacyZeroDec()
119+
totalUnsettled := sdkmath.LegacyZeroDec()
120+
109121
for idx := range payments {
110122
assert.Equal(t, sdk.NewDecCoinFromDec(denom, tt.cfg.transferred[idx]), payments[idx].State.Balance, tt.name)
123+
totalPayments.AddMut(payments[idx].State.Balance.Amount)
124+
totalPayments.AddMut(payments[idx].State.Unsettled.Amount)
125+
totalUnsettled.AddMut(payments[idx].State.Unsettled.Amount)
126+
}
127+
128+
// Check that funds were decremented by the total payments amount
129+
expectedRemainingBalance := sdkmath.LegacyNewDec(tt.cfg.balanceStart).Sub(totalPayments)
130+
131+
assert.Equal(t, expectedRemainingBalance, account.State.Funds[0].Amount, fmt.Sprintf("%s: deposit balance should be decremented by total payments", tt.name))
132+
133+
// Check unsettled amounts are tracked when overdrawn
134+
if overdrawn {
135+
// balance expected to be negative
136+
assert.True(t, account.State.Funds[0].Amount.IsNegative())
137+
138+
unsettledDiff := account.State.Funds[0].Amount.Add(totalUnsettled)
139+
assert.Equal(t, sdkmath.LegacyZeroDec().String(), unsettledDiff.String())
111140
}
112141
}
113142
}

0 commit comments

Comments
 (0)