From 0ef5c3ad077acb7f876f89ffa6e2c535af0aec39 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Fri, 24 Jul 2015 12:32:22 -0700 Subject: [PATCH] Fix returning prematurely within if(runCall){...}. Renames --- rpc/core/txs.go | 8 +-- state/execution.go | 136 +++++++++++++++++++++++++-------------------- state/state.go | 8 +++ types/tx.go | 2 +- vm/gas.go | 1 + vm/types.go | 3 + vm/vm.go | 59 +++++++++++--------- 7 files changed, 125 insertions(+), 92 deletions(-) diff --git a/rpc/core/txs.go b/rpc/core/txs.go index 7c316bcc..994b1bbe 100644 --- a/rpc/core/txs.go +++ b/rpc/core/txs.go @@ -39,11 +39,11 @@ func Call(fromAddress, toAddress, data []byte) (*ctypes.ResponseCall, error) { BlockHeight: int64(st.LastBlockHeight), BlockHash: LeftPadWord256(st.LastBlockHash), BlockTime: st.LastBlockTime.Unix(), - GasLimit: 10000000, + GasLimit: st.GetGasLimit(), } vmach := vm.NewVM(txCache, params, caller.Address, nil) - gas := int64(1000000000) + gas := st.GetGasLimit() ret, err := vmach.Call(caller, callee, callee.Code, data, 0, &gas) if err != nil { return nil, err @@ -64,11 +64,11 @@ func CallCode(fromAddress, code, data []byte) (*ctypes.ResponseCall, error) { BlockHeight: int64(st.LastBlockHeight), BlockHash: LeftPadWord256(st.LastBlockHash), BlockTime: st.LastBlockTime.Unix(), - GasLimit: 10000000, + GasLimit: st.GetGasLimit(), } vmach := vm.NewVM(txCache, params, caller.Address, nil) - gas := int64(1000000000) + gas := st.GetGasLimit() ret, err := vmach.Call(caller, callee, code, data, 0, &gas) if err != nil { return nil, err diff --git a/state/execution.go b/state/execution.go index 7b43c120..d935666e 100644 --- a/state/execution.go +++ b/state/execution.go @@ -356,10 +356,10 @@ func ExecTx(blockCache *BlockCache, tx types.Tx, runCall bool, evc events.Fireab return types.ErrTxInvalidAddress } - createAccount := len(tx.Address) == 0 - if createAccount { + createContract := len(tx.Address) == 0 + if createContract { if !hasCreateContractPermission(blockCache, inAcc) { - return fmt.Errorf("Account %X does not have Create permission", tx.Input.Address) + return fmt.Errorf("Account %X does not have CreateContract permission", tx.Input.Address) } } else { if !hasCallPermission(blockCache, inAcc) { @@ -383,13 +383,18 @@ func ExecTx(blockCache *BlockCache, tx types.Tx, runCall bool, evc events.Fireab return types.ErrTxInsufficientFunds } - if !createAccount { + if !createContract { // Validate output if len(tx.Address) != 20 { log.Info(Fmt("Destination address is not 20 bytes %X", tx.Address)) return types.ErrTxInvalidAddress } - // this may be nil if we are still in mempool and contract was created in same block as this tx + // check if its a native contract + if vm.RegisteredNativeContract(LeftPadWord256(tx.Address)) { + return fmt.Errorf("NativeContracts can not be called using CallTx. Use a contract or the appropriate tx type (eg. PermissionsTx, NameTx)") + } + + // Output account may be nil if we are still in mempool and contract was created in same block as this tx // but that's fine, because the account will be created properly when the create tx runs in the block // and then this won't return nil. otherwise, we take their fee outAcc = blockCache.GetAccount(tx.Address) @@ -400,90 +405,101 @@ func ExecTx(blockCache *BlockCache, tx types.Tx, runCall bool, evc events.Fireab // Good! value := tx.Input.Amount - tx.Fee inAcc.Sequence += 1 + inAcc.Balance -= tx.Fee + blockCache.UpdateAccount(inAcc) + // The logic in runCall MUST NOT return. if runCall { + // VM call variables var ( gas int64 = tx.GasLimit err error = nil caller *vm.Account = toVMAccount(inAcc) - callee *vm.Account = nil + callee *vm.Account = nil // initialized below code []byte = nil + ret []byte = nil txCache = NewTxCache(blockCache) params = vm.Params{ BlockHeight: int64(_s.LastBlockHeight), BlockHash: LeftPadWord256(_s.LastBlockHash), BlockTime: _s.LastBlockTime.Unix(), - GasLimit: 10000000, + GasLimit: _s.GetGasLimit(), } ) - // get or create callee - if !createAccount { - - if outAcc == nil || len(outAcc.Code) == 0 { - // check if its a native contract - if vm.RegisteredNativeContract(LeftPadWord256(tx.Address)) { - return fmt.Errorf("NativeContracts can not be called using CallTx. Use a contract or the appropriate tx type (eg. PermissionsTx, NameTx)") - } - - // if you call an account that doesn't exist - // or an account with no code then we take fees (sorry pal) - // NOTE: it's fine to create a contract and call it within one - // block (nonce will prevent re-ordering of those txs) - // but to create with one account and call with another - // you have to wait a block to avoid a re-ordering attack - // that will take your fees - inAcc.Balance -= tx.Fee - blockCache.UpdateAccount(inAcc) - if outAcc == nil { - log.Info(Fmt("Cannot find destination address %X. Deducting fee from caller", tx.Address)) - } else { - log.Info(Fmt("Attempting to call an account (%X) with no code. Deducting fee from caller", tx.Address)) - } - return types.ErrTxInvalidAddress + if !createContract && (outAcc == nil || len(outAcc.Code) == 0) { + // if you call an account that doesn't exist + // or an account with no code then we take fees (sorry pal) + // NOTE: it's fine to create a contract and call it within one + // block (nonce will prevent re-ordering of those txs) + // but to create with one contract and call with another + // you have to wait a block to avoid a re-ordering attack + // that will take your fees + if outAcc == nil { + log.Info(Fmt("%X tries to call %X but it does not exist.", + inAcc.Address, tx.Address)) + } else { + log.Info(Fmt("%X tries to call %X but code is blank.", + inAcc.Address, tx.Address)) + } + err = types.ErrTxInvalidAddress + goto CALL_COMPLETE + } + + // get or create callee + if createContract { + if HasPermission(blockCache, inAcc, ptypes.CreateContract) { + callee = txCache.CreateAccount(caller) + log.Info(Fmt("Created new contract %X", callee.Address)) + code = tx.Data + } else { + log.Info(Fmt("Error on execution: Caller %X cannot create contract", + caller.Address)) + err = types.ErrTxPermissionDenied + goto CALL_COMPLETE } - callee = toVMAccount(outAcc) - code = callee.Code - log.Info(Fmt("Calling contract %X with code %X", callee.Address, callee.Code)) } else { - callee = txCache.CreateAccount(caller) - log.Info(Fmt("Created new account %X", callee.Address)) - code = tx.Data + callee = toVMAccount(outAcc) + log.Info(Fmt("Calling contract %X with code %X", callee.Address, callee.Code)) + code = callee.Code } log.Info(Fmt("Code for this contract: %X", code)) - txCache.UpdateAccount(caller) // because we bumped nonce - txCache.UpdateAccount(callee) // so the txCache knows about the callee and the create and/or transfer takes effect - - vmach := vm.NewVM(txCache, params, caller.Address, types.TxID(_s.ChainID, tx)) - vmach.SetFireable(evc) - - // NOTE: Call() transfers the value from caller to callee iff call succeeds. - ret, err := vmach.Call(caller, callee, code, tx.Data, value, &gas) - exception := "" - if err != nil { - exception = err.Error() - // Failure. Charge the gas fee. The 'value' was otherwise not transferred. - log.Info(Fmt("Error on execution: %v", err)) - inAcc.Balance -= tx.Fee - blockCache.UpdateAccount(inAcc) - // Throw away 'txCache' which holds incomplete updates (don't sync it). - } else { - log.Info("Successful execution") - // Success - if createAccount { - callee.Code = ret + // Run VM call and sync txCache to blockCache. + { // Capture scope for goto. + // Write caller/callee to txCache. + txCache.UpdateAccount(caller) + txCache.UpdateAccount(callee) + vmach := vm.NewVM(txCache, params, caller.Address, types.TxID(_s.ChainID, tx)) + vmach.SetFireable(evc) + // NOTE: Call() transfers the value from caller to callee iff call succeeds. + ret, err = vmach.Call(caller, callee, code, tx.Data, value, &gas) + if err != nil { + // Failure. Charge the gas fee. The 'value' was otherwise not transferred. + log.Info(Fmt("Error on execution: %v", err)) + goto CALL_COMPLETE } + log.Info("Successful execution") + if createContract { + callee.Code = ret + } txCache.Sync() } + + CALL_COMPLETE: // err may or may not be nil. + // Create a receipt from the ret and whether errored. log.Notice("VM call complete", "caller", caller, "callee", callee, "return", ret, "err", err) // Fire Events for sender and receiver // a separate event will be fired from vm for each additional call if evc != nil { + exception := "" + if err != nil { + exception = err.Error() + } evc.FireEvent(types.EventStringAccInput(tx.Input.Address), types.EventMsgCallTx{tx, ret, exception}) evc.FireEvent(types.EventStringAccOutput(tx.Address), types.EventMsgCallTx{tx, ret, exception}) } @@ -493,7 +509,7 @@ func ExecTx(blockCache *BlockCache, tx types.Tx, runCall bool, evc events.Fireab // So mempool will skip the actual .Call(), // and only deduct from the caller's balance. inAcc.Balance -= value - if createAccount { + if createContract { inAcc.Sequence += 1 } blockCache.UpdateAccount(inAcc) @@ -556,7 +572,7 @@ func ExecTx(blockCache *BlockCache, tx types.Tx, runCall bool, evc events.Fireab // ensure we are owner if bytes.Compare(entry.Owner, tx.Input.Address) != 0 { log.Info(Fmt("Sender %X is trying to update a name (%s) for which he is not owner", tx.Input.Address, tx.Name)) - return types.ErrIncorrectOwner + return types.ErrTxPermissionDenied } } else { expired = true diff --git a/state/state.go b/state/state.go index da585e8a..9282dcbf 100644 --- a/state/state.go +++ b/state/state.go @@ -147,6 +147,14 @@ func (s *State) SetDB(db dbm.DB) { s.DB = db } +//------------------------------------- +// State.params + +func (s *State) GetGasLimit() int64 { + return 1000000 // TODO +} + +// State.params //------------------------------------- // State.accounts diff --git a/types/tx.go b/types/tx.go index 3ebb143e..4b87912e 100644 --- a/types/tx.go +++ b/types/tx.go @@ -21,7 +21,7 @@ var ( ErrTxInvalidPubKey = errors.New("Error invalid pubkey") ErrTxInvalidSignature = errors.New("Error invalid signature") ErrTxInvalidString = errors.New("Error invalid string") - ErrIncorrectOwner = errors.New("Error incorrect owner") + ErrTxPermissionDenied = errors.New("Error permission denied") ) type ErrTxInvalidSequence struct { diff --git a/vm/gas.go b/vm/gas.go index dac978f1..fe4a43d8 100644 --- a/vm/gas.go +++ b/vm/gas.go @@ -5,6 +5,7 @@ const ( GasGetAccount int64 = 1 GasStorageUpdate int64 = 1 + GasBaseOp int64 = 0 // TODO: make this 1 GasStackOp int64 = 1 GasEcRecover int64 = 1 diff --git a/vm/types.go b/vm/types.go index 970f152d..1a6f6c3d 100644 --- a/vm/types.go +++ b/vm/types.go @@ -21,6 +21,9 @@ type Account struct { } func (acc *Account) String() string { + if acc == nil { + return "nil-VMAccount" + } return Fmt("VMAccount{%X B:%v C:%X N:%v S:%X}", acc.Address, acc.Balance, acc.Code, acc.Nonce, acc.StorageRoot) } diff --git a/vm/vm.go b/vm/vm.go index 8c16c5b7..ee1298db 100644 --- a/vm/vm.go +++ b/vm/vm.go @@ -140,6 +140,18 @@ func (vm *VM) Call(caller, callee *Account, code, input []byte, value int64, gas return } +// Try to deduct gasToUse from gasLeft. If ok return false, otherwise +// set err and return true. +func useGasNegative(gasLeft *int64, gasToUse int64, err *error) bool { + if *gasLeft >= gasToUse { + *gasLeft -= gasToUse + return false + } else if *err == nil { + *err = ErrInsufficientGas + } + return true +} + // Just like Call() but does not transfer 'value' or modify the callDepth. func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas *int64) (output []byte, err error) { dbg.Printf("(%d) (%X) %X (code=%d) gas: %v (d) %X\n", vm.callDepth, caller.Address[:4], callee.Address, len(callee.Code), *gas, input) @@ -148,12 +160,12 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas pc int64 = 0 stack = NewStack(dataStackCapacity, gas, &err) memory = make([]byte, memoryCapacity) - ok = false // convenience ) for { - // If there is an error, return - if err != nil { + + // Use BaseOp gas. + if useGasNegative(gas, GasBaseOp, &err) { return nil, err } @@ -424,8 +436,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas dbg.Printf(" => 0x%X\n", res) case SHA3: // 0x20 - if ok = useGas(gas, GasSha3); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasSha3, &err) { + return nil, err } offset, size := stack.Pop64(), stack.Pop64() data, ok := subslice(memory, offset, size) @@ -442,8 +454,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas case BALANCE: // 0x31 addr := stack.Pop() - if ok = useGas(gas, GasGetAccount); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasGetAccount, &err) { + return nil, err } acc := vm.appState.GetAccount(addr) if acc == nil { @@ -520,8 +532,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas case EXTCODESIZE: // 0x3B addr := stack.Pop() - if ok = useGas(gas, GasGetAccount); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasGetAccount, &err) { + return nil, err } acc := vm.appState.GetAccount(addr) if acc == nil { @@ -534,8 +546,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas case EXTCODECOPY: // 0x3C addr := stack.Pop() - if ok = useGas(gas, GasGetAccount); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasGetAccount, &err) { + return nil, err } acc := vm.appState.GetAccount(addr) if acc == nil { @@ -579,8 +591,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas dbg.Printf(" => %v\n", vm.params.GasLimit) case POP: // 0x50 - stack.Pop() - dbg.Printf(" => %v\n", vm.params.GasLimit) + popped := stack.Pop() + dbg.Printf(" => 0x%X\n", popped) case MLOAD: // 0x51 offset := stack.Pop64() @@ -616,8 +628,10 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas case SSTORE: // 0x55 loc, data := stack.Pop(), stack.Pop() + if useGasNegative(gas, GasStorageUpdate, &err) { + return nil, err + } vm.appState.SetStorage(callee.Address, loc, data) - useGas(gas, GasStorageUpdate) dbg.Printf(" {0x%X : 0x%X}\n", loc, data) case JUMP: // 0x56 @@ -762,8 +776,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas vm.fireCallEvent(&exception, &ret, callee, &Account{Address: addr}, args, value, gas) } else { // EVM contract - if ok = useGas(gas, GasGetAccount); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasGetAccount, &err) { + return nil, err } acc := vm.appState.GetAccount(addr) // since CALL is used also for sending funds, @@ -821,8 +835,8 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas case SUICIDE: // 0xFF addr := stack.Pop() - if ok = useGas(gas, GasGetAccount); !ok { - return nil, firstErr(err, ErrInsufficientGas) + if useGasNegative(gas, GasGetAccount, &err) { + return nil, err } // TODO if the receiver is , then make it the fee. receiver := vm.appState.GetAccount(addr) @@ -893,15 +907,6 @@ func firstErr(errA, errB error) error { } } -func useGas(gas *int64, gasToUse int64) bool { - if *gas > gasToUse { - *gas -= gasToUse - return true - } else { - return false - } -} - func transfer(from, to *Account, amount int64) error { if from.Balance < amount { return ErrInsufficientBalance