core/vm: address review concerns

common/types.go

@@ -202,12 +202,6 @@ func IsHexAddress(s string) bool {
 // Bytes gets the string representation of the underlying address.
 func (a Address) Bytes() []byte { return a[:] }
 
-// Big converts an address to a big integer.
-func (a Address) Big() *big.Int { return new(big.Int).SetBytes(a[:]) }
-
-// SetBig converts an address to a given big integer.
-func (a Address) SetBig(int *big.Int) *big.Int { return int.SetBytes(a[:]) }
-
 // Hash converts an address to a hash by left-padding it with zeros.
 func (a Address) Hash() Hash { return BytesToHash(a[:]) }
 

common/types_test.go

@@ -114,8 +114,8 @@ func TestAddressUnmarshalJSON(t *testing.T) {
 			if test.ShouldErr {
 				t.Errorf("test #%d: expected error, got none", i)
 			}
-			if v.Big().Cmp(test.Output) != 0 {
+			if got := new(big.Int).SetBytes(v.Bytes()); got.Cmp(test.Output) != 0 {
-				t.Errorf("test #%d: address mismatch: have %v, want %v", i, v.Big(), test.Output)
+				t.Errorf("test #%d: address mismatch: have %v, want %v", i, got, test.Output)
 			}
 		}
 	}

core/mkalloc.go

@@ -52,7 +52,8 @@ func makelist(g *core.Genesis) allocList {
 		if len(account.Storage) > 0 || len(account.Code) > 0 || account.Nonce != 0 {
 			panic(fmt.Sprintf("can't encode account %x", addr))
 		}
-		a = append(a, allocItem{addr.Big(), account.Balance})
+		bigAddr := new(big.Int).SetBytes(addr.Bytes())
+		a = append(a, allocItem{bigAddr, account.Balance})
 	}
 	sort.Sort(a)
 	return a

core/vm/common.go

@@ -26,23 +26,10 @@ import (
 // calcMemSize64 calculates the required memory size, and returns
 // the size and whether the result overflowed uint64
 func calcMemSize64(off, l *big.Int) (uint64, bool) {
-	// if length is zero, memsize is always zero, regardless of offset
+	if !l.IsUint64() {
-	if l.Sign() == 0 {
-		return 0, false
-	}
-	// Check that neither offset nor length overflows
-	if off.BitLen() > 64 || l.BitLen() > 64 {
 		return 0, true
 	}
-
+	return calcMemSize64WithUint(off, l.Uint64())
-	offset64 := off.Uint64()
-	length64 := l.Uint64()
-	val := offset64 + length64
-	// Check that the total doesn't overflow
-	if val < offset64 {
-		return 0, true
-	}
-	return val, false
 }
 
 // calcMemSize64WithUint calculates the required memory size, and returns
@@ -54,16 +41,13 @@ func calcMemSize64WithUint(off *big.Int, length64 uint64) (uint64, bool) {
 		return 0, false
 	}
 	// Check that offset doesn't overflow
-	if off.BitLen() > 64 {
+	if !off.IsUint64() {
 		return 0, true
 	}
 	offset64 := off.Uint64()
 	val := offset64 + length64
-	// Check that the total doesn't overflow
+	// if value < either of it's parts, then it overflowed
-	if val < offset64 {
+	return val, val < offset64
-		return 0, true
-	}
-	return val, false
 }
 
 // getData returns a slice from the data based on the start and size and pads
@@ -93,7 +77,7 @@ func getDataBig(data []byte, start *big.Int, size *big.Int) []byte {
 // bigUint64 returns the integer casted to a uint64 and returns whether it
 // overflowed in the process.
 func bigUint64(v *big.Int) (uint64, bool) {
-	return v.Uint64(), v.BitLen() > 64
+	return v.Uint64(), !v.IsUint64()
 }
 
 // toWordSize returns the ceiled word size required for memory expansion.

core/vm/instructions.go

@@ -405,7 +405,7 @@ func opSha3(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory
 }
 
 func opAddress(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
-	stack.push(contract.Address().SetBig(interpreter.intPool.get()))
+	stack.push(interpreter.intPool.get().SetBytes(contract.Address().Bytes()))
 	return nil, nil
 }
 
@@ -416,12 +416,12 @@ func opBalance(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo
 }
 
 func opOrigin(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
-	stack.push(interpreter.evm.Origin.SetBig(interpreter.intPool.get()))
+	stack.push(interpreter.intPool.get().SetBytes(interpreter.evm.Origin.Bytes()))
 	return nil, nil
 }
 
 func opCaller(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
-	stack.push(contract.Caller().SetBig(interpreter.intPool.get()))
+	stack.push(interpreter.intPool.get().SetBytes(contract.Caller().Bytes()))
 	return nil, nil
 }
 
@@ -572,7 +572,7 @@ func opBlockhash(pc *uint64, interpreter *EVMInterpreter, contract *Contract, me
 }
 
 func opCoinbase(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
-	stack.push(interpreter.evm.Coinbase.Big())
+	stack.push(interpreter.intPool.get().SetBytes(interpreter.evm.Coinbase.Bytes()))
 	return nil, nil
 }
 
@@ -709,7 +709,7 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memor
 	} else if suberr != nil && suberr != ErrCodeStoreOutOfGas {
 		stack.push(interpreter.intPool.getZero())
 	} else {
-		stack.push(addr.Big())
+		stack.push(interpreter.intPool.get().SetBytes(addr.Bytes()))
 	}
 	contract.Gas += returnGas
 	interpreter.intPool.put(value, offset, size)
@@ -737,7 +737,7 @@ func opCreate2(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo
 	if suberr != nil {
 		stack.push(interpreter.intPool.getZero())
 	} else {
-		stack.push(addr.Big())
+		stack.push(interpreter.intPool.get().SetBytes(addr.Bytes()))
 	}
 	contract.Gas += returnGas
 	interpreter.intPool.put(endowment, offset, size, salt)
@@ -884,21 +884,6 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo
 	return nil, nil
 }
 
-// opPush1 is a specialized version of pushN
-func opPush1(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
-	var (
-		codeLen = uint64(len(contract.Code))
-		integer = interpreter.intPool.get()
-	)
-	*pc += 1
-	if *pc < codeLen {
-		stack.push(integer.SetUint64(uint64(contract.Code[*pc])))
-	} else {
-		stack.push(integer.SetUint64(0))
-	}
-	return nil, nil
-}
-
 // following functions are used by the instruction jump  table
 
 // make log instruction function
@@ -925,6 +910,21 @@ func makeLog(size int) executionFunc {
 	}
 }
 
+// opPush1 is a specialized version of pushN
+func opPush1(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
+	var (
+		codeLen = uint64(len(contract.Code))
+		integer = interpreter.intPool.get()
+	)
+	*pc += 1
+	if *pc < codeLen {
+		stack.push(integer.SetUint64(uint64(contract.Code[*pc])))
+	} else {
+		stack.push(integer.SetUint64(0))
+	}
+	return nil, nil
+}
+
 // make push instruction function
 func makePush(size uint64, pushByteSize int) executionFunc {
 	return func(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {

core/vm/interpreter.go

@@ -204,11 +204,9 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
 		// Validate stack
 		if sLen := stack.len(); sLen < operation.minStack {
 			return nil, fmt.Errorf("stack underflow (%d <=> %d)", sLen, operation.minStack)
-		} else {
+		} else if sLen > operation.maxStack {
-			if sLen > operation.maxStack {
 			return nil, fmt.Errorf("stack limit reached %d (%d)", sLen, operation.maxStack)
 		}
-		}
 		// If the operation is valid, enforce and write restrictions
 		if in.readOnly && in.evm.chainRules.IsByzantium {
 			// If the interpreter is operating in readonly mode, make sure no
@@ -228,6 +226,8 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
 		var memorySize uint64
 		// calculate the new memory size and expand the memory to fit
 		// the operation
+		// Memory check needs to be done prior to evaluating the dynamic gas portion,
+		// to detect calculation overflows
 		if operation.memorySize != nil {
 			memSize, overflow := operation.memorySize(stack)
 			if overflow {