From a8c3225ba3837701f262acff967dadd6a844e329 Mon Sep 17 00:00:00 2001 From: Silas Davis <github@silasdavis.net> Date: Wed, 5 Oct 2016 00:19:24 +0100 Subject: [PATCH] Revert "Don't use difference between caller's and callee's gas as the gas limit for the call" --- .../tendermint/tendermint/vm/test/vm_test.go | 272 +++--------------- .../github.com/tendermint/tendermint/vm/vm.go | 12 +- 2 files changed, 49 insertions(+), 235 deletions(-) diff --git a/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/test/vm_test.go b/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/test/vm_test.go index 17b40bc5..900a3ff4 100644 --- a/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/test/vm_test.go +++ b/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/test/vm_test.go @@ -8,20 +8,13 @@ import ( "testing" "time" - "errors" - . "github.com/eris-ltd/eris-db/Godeps/_workspace/src/github.com/tendermint/tendermint/common" "github.com/eris-ltd/eris-db/Godeps/_workspace/src/github.com/tendermint/tendermint/events" ptypes "github.com/eris-ltd/eris-db/Godeps/_workspace/src/github.com/tendermint/tendermint/permission/types" "github.com/eris-ltd/eris-db/Godeps/_workspace/src/github.com/tendermint/tendermint/types" . "github.com/eris-ltd/eris-db/Godeps/_workspace/src/github.com/tendermint/tendermint/vm" - "github.com/stretchr/testify/assert" ) -func init() { - SetDebug(true) -} - func newAppState() *FakeAppState { fas := &FakeAppState{ accounts: make(map[string]*Account), @@ -161,164 +154,65 @@ func TestSendCall(t *testing.T) { //---------------------------------------------- // account2 has insufficient balance, should fail - _, err := runVMWaitError(ourVm, account1, account2, addr, contractCode, 1000) - assert.Error(t, err, "Expected insufficient balance error") + fmt.Println("Should fail with insufficient balance") + + exception := runVMWaitEvents(t, ourVm, account1, account2, addr, contractCode, 1000) + if exception == "" { + t.Fatal("Expected exception") + } //---------------------------------------------- // give account2 sufficient balance, should pass + account2.Balance = 100000 - _, err = runVMWaitError(ourVm, account1, account2, addr, contractCode, 1000) - assert.NoError(t, err, "Should have sufficient balance") + exception = runVMWaitEvents(t, ourVm, account1, account2, addr, contractCode, 1000) + if exception != "" { + t.Fatal("Unexpected exception", exception) + } //---------------------------------------------- // insufficient gas, should fail + fmt.Println("Should fail with insufficient gas") account2.Balance = 100000 - _, err = runVMWaitError(ourVm, account1, account2, addr, contractCode, 100) - assert.Error(t, err, "Expected insufficient gas error") -} - -// This test was introduced to cover an issues exposed in our handling of the -// gas limit passed from caller to callee on various forms of CALL -// this ticket gives some background: https://github.com/eris-ltd/eris-pm/issues/212 -// The idea of this test is to implement a simple DelegateCall in EVM code -// We first run the DELEGATECALL with _just_ enough gas expecting a simple return, -// and then run it with 1 gas unit less, expecting a failure -func TestDelegateCallGas(t *testing.T) { - appState := newAppState() - ourVm := NewVM(appState, newParams(), Zero256, nil) - - inOff := 0 - inSize := 0 // no call data - retOff := 0 - retSize := 32 - calleeReturnValue := int64(20) - - // DELEGATECALL(retSize, refOffset, inSize, inOffset, addr, gasLimit) - // 6 pops - delegateCallCost := GasStackOp * 6 - // 1 push - gasCost := GasStackOp - // 2 pops, 1 push - subCost := GasStackOp * 3 - pushCost := GasStackOp - - costBetweenGasAndDelegateCall := gasCost + subCost + delegateCallCost + pushCost - - // Do a simple operation using 1 gas unit - calleeAccount, calleeAddress := makeAccountWithCode(appState, "callee", - bytecode(PUSH1, calleeReturnValue, return1())) - - // Here we split up the caller code so we can make a DELEGATE call with - // different amounts of gas. The value we sandwich in the middle is the amount - // we subtract from the available gas (that the caller has available), so: - // code := bytecode(callerCodePrefix, <amount to subtract from GAS> , callerCodeSuffix) - // gives us the code to make the call - callerCodePrefix := bytecode(PUSH1, retSize, PUSH1, retOff, PUSH1, inSize, - PUSH1, inOff, PUSH20, calleeAddress, PUSH1) - callerCodeSuffix := bytecode(GAS, SUB, DELEGATECALL, returnWord()) - - // Perform a delegate call - callerAccount, _ := makeAccountWithCode(appState, "caller", - bytecode(callerCodePrefix, - // Give just enough gas to make the DELEGATECALL - costBetweenGasAndDelegateCall, - callerCodeSuffix)) - - // Should pass - output, err := runVMWaitError(ourVm, callerAccount, calleeAccount, calleeAddress, - callerAccount.Code, 100) - assert.NoError(t, err, "Should have sufficient funds for call") - assert.Equal(t, Int64ToWord256(calleeReturnValue).Bytes(), output) - - callerAccount.Code = bytecode(callerCodePrefix, - // Shouldn't be enough gas to make call - costBetweenGasAndDelegateCall-1, - callerCodeSuffix) - - // Should fail - _, err = runVMWaitError(ourVm, callerAccount, calleeAccount, calleeAddress, - callerAccount.Code, 100) - assert.Error(t, err, "Should have insufficient funds for call") -} - -// Store the top element of the stack (which is a 32-byte word) in memory -// and return it. Useful for a simple return value. -func return1() []byte { - return bytecode(PUSH1, 0, MSTORE, returnWord()) -} - -func returnWord() []byte { - // PUSH1 => return size, PUSH1 => return offset, RETURN - return bytecode(PUSH1, 32, PUSH1, 0, RETURN) -} - -func makeAccountWithCode(appState AppState, name string, - code []byte) (*Account, []byte) { - account := &Account{ - Address: LeftPadWord256([]byte(name)), - Balance: 9999999, - Code: code, - Nonce: 0, - } - account.Code = code - appState.UpdateAccount(account) - // Sanity check - address := new([20]byte) - for i, b := range account.Address.Postfix(20) { - address[i] = b - } - return account, address[:] -} - -// Subscribes to an AccCall, runs the vm, returns the output any direct exception -// and then waits for any exceptions transmitted by EventData in the AccCall -// event (in the case of no direct error from call we will block waiting for -// at least 1 AccCall event) -func runVMWaitError(ourVm *VM, caller, callee *Account, subscribeAddr, - contractCode []byte, gas int64) (output []byte, err error) { - eventCh := make(chan types.EventData) - output, err = runVM(eventCh, ourVm, caller, callee, subscribeAddr, - contractCode, gas) - if err != nil { - return - } - msg := <-eventCh - var errString string - switch ev := msg.(type) { - case types.EventDataTx: - errString = ev.Exception - case types.EventDataCall: - errString = ev.Exception - } - - if errString != "" { - err = errors.New(errString) + exception = runVMWaitEvents(t, ourVm, account1, account2, addr, contractCode, 100) + if exception == "" { + t.Fatal("Expected exception") } - return } -// Subscribes to an AccCall, runs the vm, returns the output and any direct -// exception -func runVM(eventCh chan types.EventData, ourVm *VM, caller, callee *Account, - subscribeAddr, contractCode []byte, gas int64) ([]byte, error) { - +// subscribes to an AccCall, runs the vm, returns the exception +func runVMWaitEvents(t *testing.T, ourVm *VM, caller, callee *Account, subscribeAddr, contractCode []byte, gas int64) string { // we need to catch the event from the CALL to check for exceptions evsw := events.NewEventSwitch() evsw.Start() + ch := make(chan interface{}) fmt.Printf("subscribe to %x\n", subscribeAddr) - evsw.AddListenerForEvent("test", types.EventStringAccCall(subscribeAddr), - func(msg types.EventData) { - eventCh <- msg - }) + evsw.AddListenerForEvent("test", types.EventStringAccCall(subscribeAddr), func(msg types.EventData) { + ch <- msg + }) evc := events.NewEventCache(evsw) ourVm.SetFireable(evc) - start := time.Now() - output, err := ourVm.Call(caller, callee, contractCode, []byte{}, 0, &gas) - fmt.Printf("Output: %v Error: %v\n", output, err) - fmt.Println("Call took:", time.Since(start)) - go func() { evc.Flush() }() - return output, err + go func() { + start := time.Now() + output, err := ourVm.Call(caller, callee, contractCode, []byte{}, 0, &gas) + fmt.Printf("Output: %v Error: %v\n", output, err) + fmt.Println("Call took:", time.Since(start)) + if err != nil { + ch <- err.Error() + } + evc.Flush() + }() + msg := <-ch + switch ev := msg.(type) { + case types.EventDataTx: + return ev.Exception + case types.EventDataCall: + return ev.Exception + case string: + return ev + } + return "" } // this is code to call another contract (hardcoded as addr) @@ -328,92 +222,10 @@ func callContractCode(addr []byte) []byte { inOff, inSize := byte(0x0), byte(0x0) // no call data retOff, retSize := byte(0x0), byte(0x20) // this is the code we want to run (send funds to an account and return) - return bytecode(PUSH1, retSize, PUSH1, retOff, PUSH1, inSize, PUSH1, - inOff, PUSH1, value, PUSH20, addr, PUSH2, gas1, gas2, CALL, PUSH1, retSize, - PUSH1, retOff, RETURN) -} - -func TestBytecode(t *testing.T) { - assert.Equal(t, - bytecode(1, 2, 3, 4, 5, 6), - bytecode(1, 2, 3, bytecode(4, 5, 6))) - assert.Equal(t, - bytecode(1, 2, 3, 4, 5, 6, 7, 8), - bytecode(1, 2, 3, bytecode(4, bytecode(5), 6), 7, 8)) - assert.Equal(t, - bytecode(PUSH1, 2), - bytecode(byte(PUSH1), 0x02)) - assert.Equal(t, - []byte{}, - bytecode(bytecode(bytecode()))) - - contractAccount := &Account{Address: Int64ToWord256(102)} - addr := contractAccount.Address.Postfix(20) - gas1, gas2 := byte(0x1), byte(0x1) - value := byte(0x69) - inOff, inSize := byte(0x0), byte(0x0) // no call data - retOff, retSize := byte(0x0), byte(0x20) - contractCodeBytecode := bytecode(PUSH1, retSize, PUSH1, retOff, PUSH1, inSize, PUSH1, - inOff, PUSH1, value, PUSH20, addr, PUSH2, gas1, gas2, CALL, PUSH1, retSize, - PUSH1, retOff, RETURN) contractCode := []byte{0x60, retSize, 0x60, retOff, 0x60, inSize, 0x60, inOff, 0x60, value, 0x73} contractCode = append(contractCode, addr...) contractCode = append(contractCode, []byte{0x61, gas1, gas2, 0xf1, 0x60, 0x20, 0x60, 0x0, 0xf3}...) - assert.Equal(t, contractCode, contractCodeBytecode) -} - -func TestConcat(t *testing.T) { - assert.Equal(t, - []byte{0x01, 0x02, 0x03, 0x04}, - concat([]byte{0x01, 0x02}, []byte{0x03, 0x04})) -} - -// Convenience function to allow us to mix bytes, ints, and OpCodes that -// represent bytes in an EVM assembly code to make assembly more readable. -// Also allows us to splice together assembly -// fragments because any []byte arguments are flattened in the result. -func bytecode(bytelikes ...interface{}) []byte { - bytes := make([]byte, len(bytelikes)) - for i, bytelike := range bytelikes { - switch b := bytelike.(type) { - case byte: - bytes[i] = b - case OpCode: - bytes[i] = byte(b) - case int: - bytes[i] = byte(b) - if int(bytes[i]) != b { - panic(fmt.Sprintf("The int %v does not fit inside a byte", b)) - } - case int64: - bytes[i] = byte(b) - if int64(bytes[i]) != b { - panic(fmt.Sprintf("The int64 %v does not fit inside a byte", b)) - } - case []byte: - // splice - return concat(bytes[:i], b, bytecode(bytelikes[i+1:]...)) - default: - panic("Only byte-like codes (and []byte sequences) can be used to form bytecode") - } - } - return bytes -} - -func concat(bss ...[]byte) []byte { - offset := 0 - for _, bs := range bss { - offset += len(bs) - } - bytes := make([]byte, offset) - offset = 0 - for _, bs := range bss { - for i, b := range bs { - bytes[offset+i] = b - } - offset += len(bs) - } - return bytes + return contractCode } /* diff --git a/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/vm.go b/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/vm.go index e1f9c7e4..f79a3d28 100644 --- a/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/vm.go +++ b/Godeps/_workspace/src/github.com/tendermint/tendermint/vm/vm.go @@ -17,7 +17,7 @@ var ( ErrUnknownAddress = errors.New("Unknown address") ErrInsufficientBalance = errors.New("Insufficient balance") ErrInvalidJumpDest = errors.New("Invalid jump dest") - ErrInsufficientGas = errors.New("Insufficient gas") + ErrInsufficientGas = errors.New("Insuffient gas") ErrMemoryOutOfBounds = errors.New("Memory out of bounds") ErrCodeOutOfBounds = errors.New("Code out of bounds") ErrInputOutOfBounds = errors.New("Input out of bounds") @@ -194,6 +194,7 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas ) for { + // Use BaseOp gas. if useGasNegative(gas, GasBaseOp, &err) { return nil, err @@ -815,7 +816,7 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas exception = err.Error() } // NOTE: these fire call events and not particular events for eg name reg or permissions - vm.fireCallEvent(&exception, &ret, callee, &Account{Address: addr}, args, value, &gasLimit) + vm.fireCallEvent(&exception, &ret, callee, &Account{Address: addr}, args, value, gas) } else { // EVM contract if useGasNegative(gas, GasGetAccount, &err) { @@ -830,12 +831,12 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas if acc == nil { return nil, firstErr(err, ErrUnknownAddress) } - ret, err = vm.Call(callee, callee, acc.Code, args, value, &gasLimit) + ret, err = vm.Call(callee, callee, acc.Code, args, value, gas) } else if op == DELEGATECALL { if acc == nil { return nil, firstErr(err, ErrUnknownAddress) } - ret, err = vm.DelegateCall(caller, callee, acc.Code, args, value, &gasLimit) + ret, err = vm.DelegateCall(caller, callee, acc.Code, args, value, gas) } else { // nil account means we're sending funds to a new account if acc == nil { @@ -846,7 +847,7 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas } // add account to the tx cache vm.appState.UpdateAccount(acc) - ret, err = vm.Call(callee, acc, acc.Code, args, value, &gasLimit) + ret, err = vm.Call(callee, acc, acc.Code, args, value, gas) } } @@ -905,6 +906,7 @@ func (vm *VM) call(caller, callee *Account, code, input []byte, value int64, gas } pc++ + } } -- GitLab