From 227cb0bee931c3e5af9f916f5e285d45883fabc2 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Mon, 13 Oct 2014 23:34:23 -0700 Subject: [PATCH] GetAccountDetail() returns a copy. More tests. --- state/account.go | 20 +++++++- state/state.go | 13 +++-- state/state_test.go | 115 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 130 insertions(+), 18 deletions(-) diff --git a/state/account.go b/state/account.go index a4770aeaa..d099155d0 100644 --- a/state/account.go +++ b/state/account.go @@ -1,11 +1,13 @@ package state import ( + "fmt" + "io" + . "github.com/tendermint/tendermint/binary" . "github.com/tendermint/tendermint/blocks" . "github.com/tendermint/tendermint/common" "github.com/tendermint/tendermint/crypto" - "io" ) const ( @@ -54,6 +56,10 @@ func (account Account) Verify(o Signable) bool { return account.VerifyBytes(msg, sig) } +func (account Account) String() string { + return fmt.Sprintf("Account{%v:%X}", account.Id, account.PubKey) +} + //----------------------------------------------------------------------------- type AccountDetail struct { @@ -72,7 +78,7 @@ func ReadAccountDetail(r io.Reader, n *int64, err *error) *AccountDetail { } } -func (accDet AccountDetail) WriteTo(w io.Writer) (n int64, err error) { +func (accDet *AccountDetail) WriteTo(w io.Writer) (n int64, err error) { WriteBinary(w, accDet.Account, &n, &err) WriteUVarInt(w, accDet.Sequence, &n, &err) WriteUInt64(w, accDet.Balance, &n, &err) @@ -80,6 +86,16 @@ func (accDet AccountDetail) WriteTo(w io.Writer) (n int64, err error) { return } +func (accDet *AccountDetail) String() string { + return fmt.Sprintf("AccountDetail{%v:%X Sequence:%v Balance:%v Status:%X}", + accDet.Id, accDet.PubKey, accDet.Sequence, accDet.Balance, accDet.Status) +} + +func (accDet *AccountDetail) Copy() *AccountDetail { + accDetCopy := *accDet + return &accDetCopy +} + //------------------------------------- var AccountDetailCodec = accountDetailCodec{} diff --git a/state/state.go b/state/state.go index bfc9b9b55..94aa4c641 100644 --- a/state/state.go +++ b/state/state.go @@ -153,9 +153,12 @@ func (s *State) ExecTx(tx Tx) error { if !accDet.Verify(tx) { return ErrStateInvalidSignature } - // Check sequence + // Check and update sequence if tx.GetSequence() <= accDet.Sequence { return ErrStateInvalidSequenceNumber + } else { + // TODO consider prevSequence for tx chaining. + accDet.Sequence = tx.GetSequence() } // Subtract fee from balance. if accDet.Balance < tx.GetFee() { @@ -397,17 +400,21 @@ func (s *State) AppendBlock(b *Block, checkStateHash bool) error { return nil } +// The returned AccountDetail is a copy, so mutating it +// has no side effects. func (s *State) GetAccountDetail(accountId uint64) *AccountDetail { _, accDet := s.AccountDetails.Get(accountId) if accDet == nil { return nil } - return accDet.(*AccountDetail) + return accDet.(*AccountDetail).Copy() } // Returns false if new, true if updated. +// The accDet is copied before setting, so mutating it +// afterwards has no side effects. func (s *State) SetAccountDetail(accDet *AccountDetail) (updated bool) { - return s.AccountDetails.Set(accDet.Id, accDet) + return s.AccountDetails.Set(accDet.Id, accDet.Copy()) } // Returns a hash that represents the state data, diff --git a/state/state_test.go b/state/state_test.go index 0d8b686b1..ee7c0654f 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -11,37 +11,40 @@ import ( "time" ) -func randAccountDetail(id uint64, status byte) *AccountDetail { +func randAccountDetail(id uint64, status byte) (*AccountDetail, *PrivAccount) { + privAccount := GenPrivAccount() + privAccount.Id = id + account := privAccount.Account return &AccountDetail{ - Account: Account{ - Id: id, - PubKey: CRandBytes(32), - }, + Account: account, Sequence: RandUInt(), - Balance: RandUInt64(), + Balance: RandUInt64() + 1000, // At least 1000. Status: status, - } + }, privAccount } // The first numValidators accounts are validators. -func randGenesisState(numAccounts int, numValidators int) *State { +func randGenesisState(numAccounts int, numValidators int) (*State, []*PrivAccount) { db := NewMemDB() accountDetails := make([]*AccountDetail, numAccounts) + privAccounts := make([]*PrivAccount, numAccounts) for i := 0; i < numAccounts; i++ { if i < numValidators { - accountDetails[i] = randAccountDetail(uint64(i), AccountStatusNominal) + accountDetails[i], privAccounts[i] = + randAccountDetail(uint64(i), AccountStatusBonded) } else { - accountDetails[i] = randAccountDetail(uint64(i), AccountStatusBonded) + accountDetails[i], privAccounts[i] = + randAccountDetail(uint64(i), AccountStatusNominal) } } s0 := GenesisState(db, time.Now(), accountDetails) s0.Save(time.Now()) - return s0 + return s0, privAccounts } func TestCopyState(t *testing.T) { // Generate a state - s0 := randGenesisState(10, 5) + s0, _ := randGenesisState(10, 5) s0Hash := s0.Hash() if len(s0Hash) == 0 { t.Error("Expected state hash") @@ -72,7 +75,7 @@ func TestCopyState(t *testing.T) { func TestGenesisSaveLoad(t *testing.T) { // Generate a state, save & load it. - s0 := randGenesisState(10, 5) + s0, _ := randGenesisState(10, 5) // Mutate the state to append one empty block. block := &Block{ Header: Header{ @@ -153,3 +156,89 @@ func TestGenesisSaveLoad(t *testing.T) { t.Error("AccountDetail mismatch") } } + +func TestTxSequence(t *testing.T) { + + state, privAccounts := randGenesisState(3, 1) + acc1 := state.GetAccountDetail(1) // Non-validator + + // Try executing a SendTx with various sequence numbers. + stx := &SendTx{ + BaseTx: BaseTx{ + Sequence: acc1.Sequence + 1, + Fee: 0}, + To: 2, + Amount: 1, + } + + // Test a variety of sequence numbers for the tx. + // The tx should only pass when i == 1. + for i := -1; i < 3; i++ { + stx.Sequence = uint(int(acc1.Sequence) + i) + privAccounts[1].Sign(stx) + stateCopy := state.Copy() + err := stateCopy.ExecTx(stx) + if i >= 1 { + // Sequence is good. + if err != nil { + t.Errorf("Expected good sequence to pass") + } + // Check accDet.Sequence. + newAcc1 := stateCopy.GetAccountDetail(1) + if newAcc1.Sequence != stx.Sequence { + t.Errorf("Expected account sequence to change") + } + } else { + // Sequence is bad. + if err == nil { + t.Errorf("Expected bad sequence to fail") + } + // Check accDet.Sequence. (shouldn't have changed) + newAcc1 := stateCopy.GetAccountDetail(1) + if newAcc1.Sequence != acc1.Sequence { + t.Errorf("Expected account sequence to not change") + } + } + } +} + +func TestTxs(t *testing.T) { + + state, privAccounts := randGenesisState(3, 1) + + acc0 := state.GetAccountDetail(0) // Validator + //_, acc0Val := state.BondedValidators.GetById(0) + if acc0.Status != AccountStatusBonded { + t.Fatal("Expected acc0 to be bonded validator") + } + acc1 := state.GetAccountDetail(1) // Non-validator + acc2 := state.GetAccountDetail(2) // Non-validator + + // SendTx. + stx := &SendTx{ + BaseTx: BaseTx{ + Sequence: acc1.Sequence + 1, + Fee: 0}, + To: 2, + Amount: 1, + } + privAccounts[1].Sign(stx) + err := state.ExecTx(stx) + if err != nil { + t.Errorf("Got error in executing send transaction, %v", err) + } + newAcc1 := state.GetAccountDetail(1) + if acc1.Balance-1 != newAcc1.Balance { + t.Errorf("Unexpected newAcc1 balance. Expected %v, got %v", + acc1.Balance-1, newAcc1.Balance) + } + newAcc2 := state.GetAccountDetail(2) + if acc2.Balance+1 != newAcc2.Balance { + t.Errorf("Unexpected newAcc2 balance. Expected %v, got %v", + acc2.Balance+1, newAcc2.Balance) + } + + // BondTx. + // XXX more tests for other transactions. + +}