From 74070f1cacb95ccbc96d47249a0be025b6b038f3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 3 Aug 2017 19:43:37 +0200 Subject: [PATCH] Add module tests for checktx return values --- modules/coin/handler.go | 4 ++-- modules/coin/handler_test.go | 31 ++++++++++++++++++++++++++++--- modules/fee/handler_test.go | 32 +++++++++++++++++++------------- modules/roles/handler.go | 4 ++-- modules/roles/handler_test.go | 4 +++- modules/roles/middleware_test.go | 5 ++++- 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/modules/coin/handler.go b/modules/coin/handler.go index c02460cbd4..0d1010803e 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -16,9 +16,9 @@ const ( //NameCoin - name space of the coin module NameCoin = "coin" // CostSend is GasAllocation per input/output - CostSend = 10 + CostSend = uint(10) // CostCredit is GasAllocation of a credit allocation - CostCredit = 20 + CostCredit = uint(20) ) // Handler includes an accountant diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index d43e20fa4e..502f1d71a2 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -85,7 +85,7 @@ func TestHandlerValidation(t *testing.T) { } } -func TestDeliverSendTx(t *testing.T) { +func TestCheckDeliverSendTx(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -110,6 +110,7 @@ func TestDeliverSendTx(t *testing.T) { tx basecoin.Tx perms []basecoin.Actor final []money // nil for error + cost uint // gas allocated (if not error) }{ { []money{{addr1, moreCoins}}, @@ -118,6 +119,7 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, someCoins}}, + 20, }, // simple multi-sig 2 accounts to 1 { @@ -127,6 +129,7 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr3, mixedCoins)}), []basecoin.Actor{addr1, addr2}, []money{{addr1, someCoins}, {addr2, diffCoins}, {addr3, mixedCoins}}, + 30, }, // multi-sig with one account sending many times { @@ -136,6 +139,17 @@ func TestDeliverSendTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, mixedCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, mixedCoins}}, + 30, + }, + // invalid send (not enough money ) + { + []money{{addr1, moreCoins}, {addr2, someCoins}}, + NewSendTx( + []TxInput{NewTxInput(addr2, moreCoins)}, + []TxOutput{NewTxOutput(addr1, moreCoins)}), + []basecoin.Actor{addr1, addr2}, + nil, + 0, }, } @@ -150,9 +164,19 @@ func TestDeliverSendTx(t *testing.T) { } ctx := stack.MockContext("base-chain", 100).WithPermissions(tc.perms...) - _, err := h.DeliverTx(ctx, store, tc.tx, nil) + + // throw-away state for checktx + cache := store.Checkpoint() + cres, err := h.CheckTx(ctx, cache, tc.tx, nil) + // real store for delivertx + _, err2 := h.DeliverTx(ctx, store, tc.tx, nil) + if len(tc.final) > 0 { // valid assert.Nil(err, "%d: %+v", i, err) + assert.Nil(err2, "%d: %+v", i, err2) + // make sure proper gas is set + assert.Equal(uint(0), cres.GasPayment, "%d", i) + assert.Equal(tc.cost, cres.GasAllocated, "%d", i) // make sure the final balances are correct for _, f := range tc.final { acct, err := loadAccount(store, f.addr.Bytes()) @@ -160,8 +184,9 @@ func TestDeliverSendTx(t *testing.T) { assert.Equal(f.coins, acct.Coins) } } else { + // both check and deliver should fail assert.NotNil(err, "%d", i) - // TODO: make sure balances unchanged! + assert.NotNil(err2, "%d", i) } } diff --git a/modules/fee/handler_test.go b/modules/fee/handler_test.go index 3fb72abaa3..546ebba2fe 100644 --- a/modules/fee/handler_test.go +++ b/modules/fee/handler_test.go @@ -55,6 +55,8 @@ func TestFeeChecks(t *testing.T) { _, err = app2.InitState(l, store, "coin", "account", key2.MakeOption()) require.Nil(err, "%+v", err) + // feeCost is what we expect if the fee is properly paid + feeCost := coin.CostSend * 2 cases := []struct { valid bool // this is the middleware stack to test @@ -68,30 +70,32 @@ func TestFeeChecks(t *testing.T) { // expected balance after the tx left coin.Coins collected coin.Coins + // expected gas allocated + expectedCost uint }{ // make sure it works with no fee (control group) - {true, app1, act1, false, act1, zero, mixed, nil}, - {true, app1, act2, false, act2, zero, pure, nil}, + {true, app1, act1, false, act1, zero, mixed, nil, 0}, + {true, app1, act2, false, act2, zero, pure, nil, 0}, // no fee or too low is rejected - {false, app2, act1, false, act1, zero, mixed, nil}, - {false, app2, act2, false, act2, zero, pure, nil}, - {false, app2, act1, true, act1, zero, mixed, nil}, - {false, app2, act2, true, act2, atom(1), pure, nil}, + {false, app2, act1, false, act1, zero, mixed, nil, 0}, + {false, app2, act2, false, act2, zero, pure, nil, 0}, + {false, app2, act1, true, act1, zero, mixed, nil, 0}, + {false, app2, act2, true, act2, atom(1), pure, nil, 0}, // proper fees are transfered in both cases - {true, app1, act1, true, act1, atom(1), wallet(1199, 55), atoms(1)}, - {true, app2, act2, true, act2, atom(57), atoms(46600), atoms(58)}, + {true, app1, act1, true, act1, atom(1), wallet(1199, 55), atoms(1), feeCost}, + {true, app2, act2, true, act2, atom(57), atoms(46600), atoms(58), feeCost}, // // fee must be the proper type... - {false, app1, act1, true, act1, eth(5), wallet(1199, 55), atoms(58)}, + {false, app1, act1, true, act1, eth(5), wallet(1199, 55), atoms(58), 0}, // signature must match - {false, app2, act1, true, act2, atom(5), atoms(46600), atoms(58)}, + {false, app2, act1, true, act2, atom(5), atoms(46600), atoms(58), 0}, // send only works within limits - {true, app2, act1, true, act1, atom(1100), wallet(99, 55), atoms(1158)}, - {false, app2, act1, true, act1, atom(500), wallet(99, 55), atoms(1158)}, + {true, app2, act1, true, act1, atom(1100), wallet(99, 55), atoms(1158), feeCost}, + {false, app2, act1, true, act1, atom(500), wallet(99, 55), atoms(1158), 0}, } for i, tc := range cases { @@ -105,9 +109,11 @@ func TestFeeChecks(t *testing.T) { ctx := stack.MockContext("x", 1).WithPermissions(tc.signer) // call checktx... - _, err := tc.app.CheckTx(ctx, store, tx) + cres, err := tc.app.CheckTx(ctx, store, tx) if tc.valid { assert.Nil(err, "%d: %+v", i, err) + assert.EqualValues(tc.fee.Amount, cres.GasPayment) + assert.EqualValues(tc.expectedCost, cres.GasAllocated) } else { assert.NotNil(err, "%d", i) } diff --git a/modules/roles/handler.go b/modules/roles/handler.go index e7073d0344..bca582fba0 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -10,9 +10,9 @@ const ( //NameRole - name space of the roles module NameRole = "role" // CostCreate is the cost to create a new role - CostCreate = 40 + CostCreate = uint(40) // CostAssume is the cost to assume a role as part of a tx - CostAssume = 5 + CostAssume = uint(5) ) // Handler allows us to create new roles diff --git a/modules/roles/handler_test.go b/modules/roles/handler_test.go index 6d1c5dfa61..1d68592594 100644 --- a/modules/roles/handler_test.go +++ b/modules/roles/handler_test.go @@ -38,11 +38,13 @@ func TestCreateRole(t *testing.T) { store := state.NewMemKVStore() for i, tc := range cases { tx := roles.NewCreateRoleTx([]byte(tc.role), tc.min, tc.sigs) - _, err := h.CheckTx(ctx, store, tx) + cres, err := h.CheckTx(ctx, store, tx) _, err2 := h.DeliverTx(ctx, store, tx) if tc.valid { assert.Nil(err, "%d/%s: %+v", i, tc.role, err) assert.Nil(err2, "%d/%s: %+v", i, tc.role, err2) + assert.Equal(roles.CostCreate, cres.GasAllocated) + assert.Equal(uint(0), cres.GasPayment) } else { assert.NotNil(err, "%d/%s", i, tc.role) assert.NotNil(err2, "%d/%s", i, tc.role) diff --git a/modules/roles/middleware_test.go b/modules/roles/middleware_test.go index 14e792b1ad..77b8bbd732 100644 --- a/modules/roles/middleware_test.go +++ b/modules/roles/middleware_test.go @@ -93,11 +93,14 @@ func TestAssumeRole(t *testing.T) { } // try CheckTx and DeliverTx and make sure they both assert permissions - _, err := app.CheckTx(myCtx, store, tx) + cres, err := app.CheckTx(myCtx, store, tx) _, err2 := app.DeliverTx(myCtx, store, tx) if tc.valid { assert.Nil(err, "%d: %+v", i, err) assert.Nil(err2, "%d: %+v", i, err2) + // make sure we charge for each role + assert.Equal(roles.CostAssume*uint(len(tc.roles)), cres.GasAllocated) + assert.Equal(uint(0), cres.GasPayment) } else { assert.NotNil(err, "%d", i) assert.NotNil(err2, "%d", i)