From ea8aa5bbc62cb242879c65d12d645d99df269294 Mon Sep 17 00:00:00 2001 From: Silas Davis <silas@monax.io> Date: Fri, 27 Jul 2018 22:57:30 +0100 Subject: [PATCH] Handle sequence number checking and incrementation generically for all tx types Signed-off-by: Silas Davis <silas@monax.io> --- core/kernel.go | 5 +- crypto/public_key.go | 4 + execution/contexts/call_context.go | 13 ---- execution/contexts/governance_context.go | 34 ++++++++- execution/contexts/name_context.go | 8 -- execution/contexts/permissions_context.go | 15 ---- execution/contexts/send_context.go | 6 +- execution/contexts/shared.go | 52 ++----------- execution/evm/accounts.go | 11 ++- execution/execution.go | 93 ++++++++++++++--------- genesis/spec/genesis_spec.go | 2 +- genesis/spec/template_account.go | 6 +- integration/governance/governance_test.go | 66 ++++++++++++++++ txs/payload/payload.go | 27 +------ txs/payload/tx_input.go | 45 ++++++++++- txs/tx.go | 5 ++ 16 files changed, 231 insertions(+), 161 deletions(-) diff --git a/core/kernel.go b/core/kernel.go index 28ff55a7..aae2435a 100644 --- a/core/kernel.go +++ b/core/kernel.go @@ -106,10 +106,11 @@ func NewKernel(ctx context.Context, keyClient keys.KeyClient, privValidator tmTy txCodec := txs.NewAminoCodec() tmGenesisDoc := tendermint.DeriveGenesisDoc(genesisDoc) - checker := execution.NewBatchChecker(kern.State, kern.Blockchain, kern.Logger) + checker := execution.NewBatchChecker(kern.State, kern.Blockchain, keyClient, kern.Logger) kern.Emitter = event.NewEmitter(kern.Logger) - committer := execution.NewBatchCommitter(kern.State, kern.Blockchain, kern.Emitter, kern.Logger, exeOptions...) + committer := execution.NewBatchCommitter(kern.State, kern.Blockchain, kern.Emitter, keyClient, kern.Logger, + exeOptions...) kern.nodeInfo = fmt.Sprintf("Burrow_%s_%X", genesisDoc.ChainID(), privValidator.GetAddress()) app := abci.NewApp(kern.nodeInfo, kern.Blockchain, checker, committer, txCodec, kern.Panic, logger) diff --git a/crypto/public_key.go b/crypto/public_key.go index 171d86ce..58203830 100644 --- a/crypto/public_key.go +++ b/crypto/public_key.go @@ -32,6 +32,10 @@ func PublicKeyLength(curveType CurveType) int { } } +func (p PublicKey) IsSet() bool { + return p.CurveType != CurveTypeUnset && p.IsValid() +} + func (p PublicKey) MarshalJSON() ([]byte, error) { jStruct := PublicKeyJSON{ CurveType: p.CurveType.String(), diff --git a/execution/contexts/call_context.go b/execution/contexts/call_context.go index 385d05e1..2dec797d 100644 --- a/execution/contexts/call_context.go +++ b/execution/contexts/call_context.go @@ -61,25 +61,12 @@ func (ctx *CallContext) Precheck() (*acm.MutableAccount, acm.Account, error) { return nil, nil, errors.ErrorCodeInvalidAddress } - err = validateInput(inAcc, ctx.tx.Input) - if err != nil { - ctx.Logger.InfoMsg("validateInput failed", - "tx_input", ctx.tx.Input, structure.ErrorKey, err) - return nil, nil, err - } if ctx.tx.Input.Amount < ctx.tx.Fee { ctx.Logger.InfoMsg("Sender did not send enough to cover the fee", "tx_input", ctx.tx.Input) return nil, nil, errors.ErrorCodeInsufficientFunds } - ctx.Logger.TraceMsg("Incrementing sequence number for CallTx", - "tag", "sequence", - "account", inAcc.Address(), - "old_sequence", inAcc.Sequence(), - "new_sequence", inAcc.Sequence()+1) - - inAcc.IncSequence() err = inAcc.SubtractFromBalance(ctx.tx.Fee) if err != nil { return nil, nil, err diff --git a/execution/contexts/governance_context.go b/execution/contexts/governance_context.go index b53a78c9..f4455a0e 100644 --- a/execution/contexts/governance_context.go +++ b/execution/contexts/governance_context.go @@ -7,9 +7,11 @@ import ( "github.com/hyperledger/burrow/acm" "github.com/hyperledger/burrow/acm/state" "github.com/hyperledger/burrow/acm/validator" + "github.com/hyperledger/burrow/crypto" "github.com/hyperledger/burrow/execution/errors" "github.com/hyperledger/burrow/execution/exec" "github.com/hyperledger/burrow/genesis/spec" + "github.com/hyperledger/burrow/keys" "github.com/hyperledger/burrow/logging" "github.com/hyperledger/burrow/permission" "github.com/hyperledger/burrow/txs/payload" @@ -18,6 +20,7 @@ import ( type GovernanceContext struct { StateWriter state.ReaderWriter ValidatorSet validator.Writer + KeyClient keys.KeyClient Logger *logging.Logger tx *payload.GovTx txe *exec.TxExecution @@ -32,7 +35,8 @@ func (ctx *GovernanceContext) Execute(txe *exec.TxExecution) error { if !ok { return fmt.Errorf("payload must be NameTx, but is: %v", txe.Envelope.Tx.Payload) } - accounts, err := getInputs(ctx.StateWriter, ctx.tx.Inputs) + // Nothing down with any incoming funds at this point + accounts, _, err := getInputs(ctx.StateWriter, ctx.tx.Inputs) if err != nil { return err } @@ -53,6 +57,13 @@ func (ctx *GovernanceContext) Execute(txe *exec.TxExecution) error { return fmt.Errorf("could not execution GovTx since account template %v contains neither "+ "address or public key", update) } + if update.PublicKey == nil { + update.PublicKey, err = ctx.MaybeGetPublicKey(*update.Address) + if err != nil { + return err + } + } + // Check address if update.PublicKey != nil { address := update.PublicKey.Address() if update.Address != nil && address != *update.Address { @@ -60,8 +71,7 @@ func (ctx *GovernanceContext) Execute(txe *exec.TxExecution) error { "GovTx", update.PublicKey, address, update.Address) } update.Address = &address - } - if update.PublicKey == nil && update.Balances().HasPower() { + } else if update.Balances().HasPower() { // If we are updating power we will need the key return fmt.Errorf("GovTx must be provided with public key when updating validator power") } @@ -132,3 +142,21 @@ func (ctx *GovernanceContext) UpdateAccount(account *acm.MutableAccount, update err = ctx.StateWriter.UpdateAccount(account) return } + +func (ctx *GovernanceContext) MaybeGetPublicKey(address crypto.Address) (*crypto.PublicKey, error) { + // First try state in case chain has received input previously + acc, err := ctx.StateWriter.GetAccount(address) + if err != nil { + return nil, err + } + if acc != nil && acc.PublicKey().IsSet() { + publicKey := acc.PublicKey() + return &publicKey, nil + } + // Then try key client + publicKey, err := ctx.KeyClient.PublicKey(address) + if err == nil { + return &publicKey, nil + } + return nil, nil +} diff --git a/execution/contexts/name_context.go b/execution/contexts/name_context.go index 10c37a15..a2215b0b 100644 --- a/execution/contexts/name_context.go +++ b/execution/contexts/name_context.go @@ -11,7 +11,6 @@ import ( "github.com/hyperledger/burrow/execution/exec" "github.com/hyperledger/burrow/execution/names" "github.com/hyperledger/burrow/logging" - "github.com/hyperledger/burrow/logging/structure" "github.com/hyperledger/burrow/txs/payload" ) @@ -48,12 +47,6 @@ func (ctx *NameContext) Execute(txe *exec.TxExecution) error { if !hasNamePermission(ctx.StateWriter, inAcc, ctx.Logger) { return fmt.Errorf("account %s does not have Name permission", ctx.tx.Input.Address) } - err = validateInput(inAcc, ctx.tx.Input) - if err != nil { - ctx.Logger.InfoMsg("validateInput failed", - "tx_input", ctx.tx.Input, structure.ErrorKey, err) - return err - } if ctx.tx.Input.Amount < ctx.tx.Fee { ctx.Logger.InfoMsg("Sender did not send enough to cover the fee", "tx_input", ctx.tx.Input) @@ -172,7 +165,6 @@ func (ctx *NameContext) Execute(txe *exec.TxExecution) error { "account", inAcc.Address(), "old_sequence", inAcc.Sequence(), "new_sequence", inAcc.Sequence()+1) - inAcc.IncSequence() err = inAcc.SubtractFromBalance(value) if err != nil { return err diff --git a/execution/contexts/permissions_context.go b/execution/contexts/permissions_context.go index ac4c450b..dc1f9949 100644 --- a/execution/contexts/permissions_context.go +++ b/execution/contexts/permissions_context.go @@ -10,7 +10,6 @@ import ( "github.com/hyperledger/burrow/execution/errors" "github.com/hyperledger/burrow/execution/exec" "github.com/hyperledger/burrow/logging" - "github.com/hyperledger/burrow/logging/structure" "github.com/hyperledger/burrow/permission" "github.com/hyperledger/burrow/txs/payload" ) @@ -51,14 +50,6 @@ func (ctx *PermissionsContext) Execute(txe *exec.TxExecution) error { permFlag.String(), permFlag) } - err = validateInput(inAcc, ctx.tx.Input) - if err != nil { - ctx.Logger.InfoMsg("validateInput failed", - "tx_input", ctx.tx.Input, - structure.ErrorKey, err) - return err - } - value := ctx.tx.Input.Amount ctx.Logger.TraceMsg("New PermsTx", @@ -114,12 +105,6 @@ func (ctx *PermissionsContext) Execute(txe *exec.TxExecution) error { } // Good! - ctx.Logger.TraceMsg("Incrementing sequence number for PermsTx", - "tag", "sequence", - "account", inAcc.Address(), - "old_sequence", inAcc.Sequence(), - "new_sequence", inAcc.Sequence()+1) - inAcc.IncSequence() err = inAcc.SubtractFromBalance(value) if err != nil { return err diff --git a/execution/contexts/send_context.go b/execution/contexts/send_context.go index a50c0b88..c9c3b5a3 100644 --- a/execution/contexts/send_context.go +++ b/execution/contexts/send_context.go @@ -25,7 +25,7 @@ func (ctx *SendContext) Execute(txe *exec.TxExecution) error { if !ok { return fmt.Errorf("payload must be NameTx, but is: %v", txe.Envelope.Tx.Payload) } - accounts, err := getInputs(ctx.StateWriter, ctx.tx.Inputs) + accounts, inTotal, err := getInputs(ctx.StateWriter, ctx.tx.Inputs) if err != nil { return err } @@ -43,10 +43,6 @@ func (ctx *SendContext) Execute(txe *exec.TxExecution) error { return err } - inTotal, err := validateInputs(accounts, ctx.tx.Inputs) - if err != nil { - return err - } outTotal, err := validateOutputs(ctx.tx.Outputs) if err != nil { return err diff --git a/execution/contexts/shared.go b/execution/contexts/shared.go index 77a11974..b33f3226 100644 --- a/execution/contexts/shared.go +++ b/execution/contexts/shared.go @@ -17,25 +17,25 @@ import ( // acm.PublicKey().(type) != nil, (it must be known), // or it must be specified in the TxInput. If redeclared, // the TxInput is modified and input.PublicKey() set to nil. -func getInputs(accountGetter state.AccountGetter, - ins []*payload.TxInput) (map[crypto.Address]*acm.MutableAccount, error) { - +func getInputs(accountGetter state.AccountGetter, ins []*payload.TxInput) (map[crypto.Address]*acm.MutableAccount, uint64, error) { + var total uint64 accounts := map[crypto.Address]*acm.MutableAccount{} for _, in := range ins { // Account shouldn't be duplicated if _, ok := accounts[in.Address]; ok { - return nil, errors.ErrorCodeDuplicateAddress + return nil, total, errors.ErrorCodeDuplicateAddress } acc, err := state.GetMutableAccount(accountGetter, in.Address) if err != nil { - return nil, err + return nil, total, err } if acc == nil { - return nil, errors.ErrorCodeInvalidAddress + return nil, total, errors.ErrorCodeInvalidAddress } accounts[in.Address] = acc + total += in.Amount } - return accounts, nil + return accounts, total, nil } func getOrMakeOutputs(accountGetter state.AccountGetter, accs map[crypto.Address]*acm.MutableAccount, @@ -82,38 +82,6 @@ func getOrMakeOutput(accountGetter state.AccountGetter, accs map[crypto.Address] return acc, nil } -func validateInputs(accs map[crypto.Address]*acm.MutableAccount, ins []*payload.TxInput) (uint64, error) { - total := uint64(0) - for _, in := range ins { - acc := accs[in.Address] - if acc == nil { - return 0, fmt.Errorf("validateInputs() expects account in accounts, but account %s not found", in.Address) - } - err := validateInput(acc, in) - if err != nil { - return 0, err - } - // Good. Add amount to total - total += in.Amount - } - return total, nil -} - -func validateInput(acc *acm.MutableAccount, in *payload.TxInput) error { - // Check sequences - if acc.Sequence()+1 != uint64(in.Sequence) { - return payload.ErrTxInvalidSequence{ - Input: in, - Account: acc, - } - } - // Check amount - if acc.Balance() < uint64(in.Amount) { - return errors.ErrorCodeInsufficientFunds - } - return nil -} - func validateOutputs(outs []*payload.TxOutput) (uint64, error) { total := uint64(0) for _, out := range outs { @@ -137,12 +105,6 @@ func adjustByInputs(accs map[crypto.Address]*acm.MutableAccount, ins []*payload. if err != nil { return err } - logger.TraceMsg("Incrementing sequence number for SendTx (adjustByInputs)", - "tag", "sequence", - "account", acc.Address(), - "old_sequence", acc.Sequence(), - "new_sequence", acc.Sequence()+1) - acc.IncSequence() } return nil } diff --git a/execution/evm/accounts.go b/execution/evm/accounts.go index 65c578e3..ad4ffae3 100644 --- a/execution/evm/accounts.go +++ b/execution/evm/accounts.go @@ -11,16 +11,15 @@ import ( // sequence number incremented func DeriveNewAccount(creator *acm.MutableAccount, permissions permission.AccountPermissions, logger *logging.Logger) *acm.MutableAccount { - // Generate an address - sequence := creator.Sequence() + logger.TraceMsg("Incrementing sequence number in DeriveNewAccount()", "tag", "sequence", "account", creator.Address(), - "old_sequence", sequence, - "new_sequence", sequence+1) - creator.IncSequence() + "old_sequence", creator.Sequence(), + "new_sequence", creator.Sequence()+1) - addr := crypto.NewContractAddress(creator.Address(), sequence) + creator.IncSequence() + addr := crypto.NewContractAddress(creator.Address(), creator.Sequence()) // Create account from address. return acm.ConcreteAccount{ diff --git a/execution/execution.go b/execution/execution.go index 5547075b..25f1da19 100644 --- a/execution/execution.go +++ b/execution/execution.go @@ -31,6 +31,7 @@ import ( "github.com/hyperledger/burrow/execution/evm" "github.com/hyperledger/burrow/execution/exec" "github.com/hyperledger/burrow/execution/names" + "github.com/hyperledger/burrow/keys" "github.com/hyperledger/burrow/logging" "github.com/hyperledger/burrow/logging/structure" "github.com/hyperledger/burrow/txs" @@ -87,7 +88,7 @@ type executor struct { var _ BatchExecutor = (*executor)(nil) // Wraps a cache of what is variously known as the 'check cache' and 'mempool' -func NewBatchChecker(backend ExecutorState, blockchain *bcm.Blockchain, logger *logging.Logger, +func NewBatchChecker(backend ExecutorState, blockchain *bcm.Blockchain, keyClient keys.KeyClient, logger *logging.Logger, options ...ExecutionOption) BatchExecutor { exe := newExecutor("CheckCache", false, backend, blockchain, event.NewNoOpPublisher(), @@ -97,13 +98,14 @@ func NewBatchChecker(backend ExecutorState, blockchain *bcm.Blockchain, logger * &contexts.GovernanceContext{ ValidatorSet: exe.blockchain.ValidatorChecker(), StateWriter: exe.stateCache, + KeyClient: keyClient, Logger: exe.logger, }, ) } -func NewBatchCommitter(backend ExecutorState, blockchain *bcm.Blockchain, emitter event.Publisher, logger *logging.Logger, - options ...ExecutionOption) BatchCommitter { +func NewBatchCommitter(backend ExecutorState, blockchain *bcm.Blockchain, emitter event.Publisher, + keyClient keys.KeyClient, logger *logging.Logger, options ...ExecutionOption) BatchCommitter { exe := newExecutor("CommitCache", true, backend, blockchain, emitter, logger.WithScope("NewBatchCommitter"), options...) @@ -112,6 +114,7 @@ func NewBatchCommitter(backend ExecutorState, blockchain *bcm.Blockchain, emitte &contexts.GovernanceContext{ ValidatorSet: exe.blockchain.ValidatorWriter(), StateWriter: exe.stateCache, + KeyClient: keyClient, Logger: exe.logger, }, ) @@ -189,50 +192,26 @@ func (exe *executor) Execute(txEnv *txs.Envelope) (txe *exec.TxExecution, err er return nil, err } - // Initialise public keys for accounts we have seen - for _, sig := range txEnv.Signatories { - // pointer dereferences are safe since txEnv.Validate() is run by txEnv.Verify() above which checks they are - // non-nil - acc, err := state.GetMutableAccount(exe.stateCache, *sig.Address) - if err != nil { - return nil, fmt.Errorf("error getting account on which to set public key: %v", *sig.Address) - } - acc.SetPublicKey(*sig.PublicKey) - err = exe.stateCache.UpdateAccount(acc) - if err != nil { - return nil, fmt.Errorf("error updating account after setting public key: %v", err) - } - } - if txExecutor, ok := exe.contexts[txEnv.Tx.Type()]; ok { // Establish new TxExecution txe := exe.blockExecution.Tx(txEnv) + // Validate inputs and check sequence numbers + err = txEnv.Tx.ValidateInputs(exe.stateCache) + if err != nil { + return nil, err + } err = txExecutor.Execute(txe) if err != nil { return nil, err } + // Initialise public keys and increment sequence numbers for Tx inputs + exe.updateSignatories(txEnv) // Return execution for this tx return txe, nil } return nil, fmt.Errorf("unknown transaction type: %v", txEnv.Tx.Type()) } -func (exe *executor) finaliseBlockExecution(header *abciTypes.Header) (*exec.BlockExecution, error) { - if header != nil && uint64(header.Height) != exe.blockExecution.Height { - return nil, fmt.Errorf("trying to finalise block execution with height %v but passed Tendermint"+ - "block header at height %v", exe.blockExecution.Height, header.Height) - } - // Capture BlockExecution to return - be := exe.blockExecution - // Set the header when provided - be.BlockHeader = exec.BlockHeaderFromHeader(header) - // Start new execution for the next height - exe.blockExecution = &exec.BlockExecution{ - Height: exe.blockExecution.Height + 1, - } - return be, nil -} - func (exe *executor) Commit(blockHash []byte, blockTime time.Time, header *abciTypes.Header) (_ []byte, err error) { // The write lock to the executor is controlled by the caller (e.g. abci.App) so we do not acquire it here to avoid @@ -319,3 +298,49 @@ func (exe *executor) GetStorage(address crypto.Address, key binary.Word256) (bin defer exe.RUnlock() return exe.stateCache.GetStorage(address, key) } + +func (exe *executor) finaliseBlockExecution(header *abciTypes.Header) (*exec.BlockExecution, error) { + if header != nil && uint64(header.Height) != exe.blockExecution.Height { + return nil, fmt.Errorf("trying to finalise block execution with height %v but passed Tendermint"+ + "block header at height %v", exe.blockExecution.Height, header.Height) + } + // Capture BlockExecution to return + be := exe.blockExecution + // Set the header when provided + be.BlockHeader = exec.BlockHeaderFromHeader(header) + // Start new execution for the next height + exe.blockExecution = &exec.BlockExecution{ + Height: exe.blockExecution.Height + 1, + } + return be, nil +} + +// Capture public keys and update sequence numbers +func (exe *executor) updateSignatories(txEnv *txs.Envelope) error { + for _, sig := range txEnv.Signatories { + // pointer dereferences are safe since txEnv.Validate() is run by txEnv.Verify() above which checks they are + // non-nil + acc, err := state.GetMutableAccount(exe.stateCache, *sig.Address) + if err != nil { + return fmt.Errorf("error getting account on which to set public key: %v", *sig.Address) + } + // Important that verify has been run against signatories at this point + if sig.PublicKey.Address() != acc.Address() { + return fmt.Errorf("unexpected mismatch between address %v and supplied public key %v", + acc.Address(), sig.PublicKey) + } + acc.SetPublicKey(*sig.PublicKey) + + exe.logger.TraceMsg("Incrementing sequence number Tx signatory/input", + "tag", "sequence", + "account", acc.Address(), + "old_sequence", acc.Sequence(), + "new_sequence", acc.Sequence()+1) + acc.IncSequence() + err = exe.stateCache.UpdateAccount(acc) + if err != nil { + return fmt.Errorf("error updating account after setting public key: %v", err) + } + } + return nil +} diff --git a/genesis/spec/genesis_spec.go b/genesis/spec/genesis_spec.go index 6990afa1..b660a4c1 100644 --- a/genesis/spec/genesis_spec.go +++ b/genesis/spec/genesis_spec.go @@ -32,7 +32,7 @@ type GenesisSpec struct { func (gs *GenesisSpec) RealiseKeys(keyClient keys.KeyClient) error { for _, templateAccount := range gs.Accounts { - _, _, err := templateAccount.RealisePubKeyAndAddress(keyClient) + _, _, err := templateAccount.RealisePublicKeyAndAddress(keyClient) if err != nil { return err } diff --git a/genesis/spec/template_account.go b/genesis/spec/template_account.go index 152a8097..fb5c55f9 100644 --- a/genesis/spec/template_account.go +++ b/genesis/spec/template_account.go @@ -13,7 +13,7 @@ import ( func (ta TemplateAccount) Validator(keyClient keys.KeyClient, index int, generateNodeKeys bool) (*genesis.Validator, error) { var err error gv := new(genesis.Validator) - gv.PublicKey, gv.Address, err = ta.RealisePubKeyAndAddress(keyClient) + gv.PublicKey, gv.Address, err = ta.RealisePublicKeyAndAddress(keyClient) if err != nil { return nil, err } @@ -55,7 +55,7 @@ func (ta TemplateAccount) AccountPermissions() (permission.AccountPermissions, e func (ta TemplateAccount) GenesisAccount(keyClient keys.KeyClient, index int) (*genesis.Account, error) { var err error ga := new(genesis.Account) - ga.PublicKey, ga.Address, err = ta.RealisePubKeyAndAddress(keyClient) + ga.PublicKey, ga.Address, err = ta.RealisePublicKeyAndAddress(keyClient) if err != nil { return nil, err } @@ -78,7 +78,7 @@ func (ta TemplateAccount) GenesisAccount(keyClient keys.KeyClient, index int) (* // Adds a public key and address to the template. If PublicKey will try to fetch it by Address. // If both PublicKey and Address are not set will use the keyClient to generate a new keypair -func (ta TemplateAccount) RealisePubKeyAndAddress(keyClient keys.KeyClient) (pubKey crypto.PublicKey, address crypto.Address, err error) { +func (ta TemplateAccount) RealisePublicKeyAndAddress(keyClient keys.KeyClient) (pubKey crypto.PublicKey, address crypto.Address, err error) { if ta.PublicKey == nil { if ta.Address == nil { // If neither PublicKey or Address set then generate a new one diff --git a/integration/governance/governance_test.go b/integration/governance/governance_test.go index c8f5c972..594d4e47 100644 --- a/integration/governance/governance_test.go +++ b/integration/governance/governance_test.go @@ -14,12 +14,14 @@ import ( "github.com/hyperledger/burrow/crypto" "github.com/hyperledger/burrow/execution/errors" "github.com/hyperledger/burrow/execution/exec" + "github.com/hyperledger/burrow/genesis/spec" "github.com/hyperledger/burrow/governance" "github.com/hyperledger/burrow/integration/rpctest" "github.com/hyperledger/burrow/permission" "github.com/hyperledger/burrow/rpc/rpcevents" "github.com/hyperledger/burrow/rpc/rpcquery" "github.com/hyperledger/burrow/rpc/rpctransact" + "github.com/hyperledger/burrow/txs" "github.com/hyperledger/burrow/txs/payload" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -155,6 +157,54 @@ func TestCreateAccount(t *testing.T) { assert.Equal(t, amount, ca.Balance) } +func TestChangePowerByAddress(t *testing.T) { + // Should use the key client to look up public key + inputAddress := privateAccounts[0].Address() + grpcAddress := testConfigs[0].RPC.GRPC.ListenAddress + tcli := rpctest.NewTransactClient(t, grpcAddress) + qcli := rpctest.NewQueryClient(t, grpcAddress) + + acc := account(2) + address := acc.Address() + power := uint64(2445) + govSync(tcli, governance.UpdateAccountTx(inputAddress, &spec.TemplateAccount{ + Address: &address, + Amounts: balance.New().Power(power), + })) + + vs, err := qcli.GetValidatorSet(context.Background(), &rpcquery.GetValidatorSetParam{}) + require.NoError(t, err) + set := validator.UnpersistSet(vs.Set) + assert.Equal(t, new(big.Int).SetUint64(power), set.Power(acc)) +} + +func TestInvalidSequenceNumber(t *testing.T) { + inputAddress := privateAccounts[0].Address() + tcli1 := rpctest.NewTransactClient(t, testConfigs[0].RPC.GRPC.ListenAddress) + tcli2 := rpctest.NewTransactClient(t, testConfigs[4].RPC.GRPC.ListenAddress) + qcli := rpctest.NewQueryClient(t, testConfigs[0].RPC.GRPC.ListenAddress) + + acc := account(2) + address := acc.Address() + power := uint64(2445) + tx := governance.UpdateAccountTx(inputAddress, &spec.TemplateAccount{ + Address: &address, + Amounts: balance.New().Power(power), + }) + + setSequence(t, qcli, tx) + _, err := localSignAndBroadcastSync(t, tcli1, tx) + require.NoError(t, err) + + // Make it a different Tx hash so it can enter cache but keep sequence number + tx.AccountUpdates[0].Amounts = balance.New().Power(power).Native(1) + _, err = localSignAndBroadcastSync(t, tcli2, tx) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid sequence") +} + +// Helpers + func getMaxFlow(t testing.TB, qcli rpcquery.QueryClient) uint64 { vs, err := qcli.GetValidatorSet(context.Background(), &rpcquery.GetValidatorSetParam{}) require.NoError(t, err) @@ -194,6 +244,22 @@ func alterPower(vs *validator.Set, i int, power uint64) { vs.AlterPower(account(i), new(big.Int).SetUint64(power)) } +func setSequence(t testing.TB, qcli rpcquery.QueryClient, tx payload.Payload) { + for _, input := range tx.GetInputs() { + ca, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: input.Address}) + require.NoError(t, err) + input.Sequence = ca.Sequence + 1 + } +} + +func localSignAndBroadcastSync(t testing.TB, tcli rpctransact.TransactClient, tx payload.Payload) (*exec.TxExecution, error) { + txEnv := txs.Enclose(genesisDoc.ChainID(), tx) + err := txEnv.Sign(privateAccounts[0]) + require.NoError(t, err) + + return tcli.BroadcastTxSync(context.Background(), &rpctransact.TxEnvelopeParam{Envelope: txEnv}) +} + func waitNBlocks(t testing.TB, ecli rpcevents.ExecutionEventsClient, n int) { stream, err := ecli.GetBlocks(context.Background(), &rpcevents.BlocksRequest{ BlockRange: rpcevents.NewBlockRange(rpcevents.LatestBound(), rpcevents.StreamBound()), diff --git a/txs/payload/payload.go b/txs/payload/payload.go index d55a8cad..71404eec 100644 --- a/txs/payload/payload.go +++ b/txs/payload/payload.go @@ -1,6 +1,8 @@ package payload -import "fmt" +import ( + "fmt" +) /* Payload (Transaction) is an atomic operation on the ledger state. @@ -95,29 +97,6 @@ type Payload interface { Size() int } -type UnknownTx struct { -} - -func (UnknownTx) String() string { - panic("implement me") -} - -func (UnknownTx) GetInputs() []*TxInput { - panic("implement me") -} - -func (UnknownTx) Type() Type { - panic("implement me") -} - -func (UnknownTx) Any() *Any { - panic("implement me") -} - -func (UnknownTx) Size() int { - panic("implement me") -} - func New(txType Type) (Payload, error) { switch txType { case TypeSend: diff --git a/txs/payload/tx_input.go b/txs/payload/tx_input.go index ff02d7cf..b3367aca 100644 --- a/txs/payload/tx_input.go +++ b/txs/payload/tx_input.go @@ -2,8 +2,49 @@ package payload import ( "fmt" + + "github.com/hyperledger/burrow/acm" + "github.com/hyperledger/burrow/acm/state" + "github.com/hyperledger/burrow/execution/errors" ) -func (txIn *TxInput) String() string { - return fmt.Sprintf("TxInput{%s, Amount: %v, Sequence:%v}", txIn.Address, txIn.Amount, txIn.Sequence) +func (input *TxInput) String() string { + return fmt.Sprintf("TxInput{%s, Amount: %v, Sequence:%v}", input.Address, input.Amount, input.Sequence) +} + +func (input *TxInput) Validate(acc acm.Account) error { + if input.Address != acc.Address() { + return fmt.Errorf("trying to validate input from address %v but passed account %v", input.Address, + acc.Address()) + } + // Check sequences + if acc.Sequence()+1 != uint64(input.Sequence) { + return ErrTxInvalidSequence{ + Input: input, + Account: acc, + } + } + // Check amount + if acc.Balance() < uint64(input.Amount) { + return errors.ErrorCodeInsufficientFunds + } + return nil +} + +func ValidateInputs(getter state.AccountGetter, ins []*TxInput) error { + for _, in := range ins { + acc, err := getter.GetAccount(in.Address) + if err != nil { + return err + } + if acc == nil { + return fmt.Errorf("validateInputs() expects to be able to retrive accoutn %v but it was not found", + in.Address) + } + err = in.Validate(acc) + if err != nil { + return err + } + } + return nil } diff --git a/txs/tx.go b/txs/tx.go index 45b15ca8..0f1c2e16 100644 --- a/txs/tx.go +++ b/txs/tx.go @@ -19,6 +19,7 @@ import ( "fmt" "github.com/hyperledger/burrow/acm" + "github.com/hyperledger/burrow/acm/state" "github.com/hyperledger/burrow/binary" "github.com/hyperledger/burrow/crypto" "github.com/hyperledger/burrow/event/query" @@ -76,6 +77,10 @@ func (tx *Tx) SignBytes() ([]byte, error) { return bs, nil } +func (tx *Tx) ValidateInputs(getter state.AccountGetter) error { + return payload.ValidateInputs(getter, tx.GetInputs()) +} + // Serialisation intermediate for switching on type type wrapper struct { ChainID string -- GitLab