Fix returning prematurely within if(runCall){...}.

Renames

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

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 !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
+			}
 
-					// 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
+			// 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
 				}
+			} else {
 				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
+				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 {
+			// 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")
-				// Success
-				if createAccount {
+				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

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
 

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 {

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

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)
 }

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