[release/1.4.7] accounts/abi: Negative numbers not properly converted in ABI encoding

When converting a negative number e.g., -2, the resulting ABI encoding
should look as follows:
fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe.
However, since the check of the type is for an uint instead of an
int, it results in the following ABI encoding:
0101010101010101010101010101010101010101010101010101010101010102. The
Ethereum ABI
(https://github.com/ethereum/wiki/wiki/Ethereum-Contract-ABI) says,
that signed integers are stored in two's complement which should be
of the form ffffff.... and not 01010101..... for e.g. -1. Thus, I
removed the type check in numbers.go as well as the function S256
as I don't think they are correct. Or maybe I'm missing something?

(cherry picked from commit 89c6c5b)

Author: tbocek

accounts/abi/method.go

@@ -62,7 +62,7 @@ func (m Method) pack(method Method, args ...interface{}) ([]byte, error) {
 			// calculate the offset
 			offset := len(method.Inputs)*32 + len(variableInput)
 			// set the offset
-			ret = append(ret, packNum(reflect.ValueOf(offset), UintTy)...)
+			ret = append(ret, packNum(reflect.ValueOf(offset))...)
 			// Append the packed output to the variable input. The variable input
 			// will be appended at the end of the input.
 			variableInput = append(variableInput, packed...)

accounts/abi/numbers.go

@@ -61,54 +61,20 @@ func U256(n *big.Int) []byte {
 	return common.LeftPadBytes(common.U256(n).Bytes(), 32)
 }
 
-func S256(n *big.Int) []byte {
-	sint := common.S256(n)
-	ret := common.LeftPadBytes(sint.Bytes(), 32)
-	if sint.Cmp(common.Big0) < 0 {
-		for i, b := range ret {
-			if b == 0 {
-				ret[i] = 1
-				continue
-			}
-			break
-		}
-	}
-
-	return ret
-}
-
 // S256 will ensure signed 256bit on big nums
 func U2U256(n uint64) []byte {
 	return U256(big.NewInt(int64(n)))
 }
 
-func S2S256(n int64) []byte {
-	return S256(big.NewInt(n))
-}
-
 // packNum packs the given number (using the reflect value) and will cast it to appropriate number representation
-func packNum(value reflect.Value, to byte) []byte {
+func packNum(value reflect.Value) []byte {
 	switch kind := value.Kind(); kind {
 	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
-		if to == UintTy {
-			return U2U256(value.Uint())
-		} else {
-			return S2S256(int64(value.Uint()))
-		}
+		return U2U256(value.Uint())
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		if to == UintTy {
-			return U2U256(uint64(value.Int()))
-		} else {
-			return S2S256(value.Int())
-		}
+		return U2U256(uint64(value.Int()))
 	case reflect.Ptr:
-		// This only takes care of packing and casting. No type checking is done here. It should be done prior to using this function.
-		if to == UintTy {
-			return U256(value.Interface().(*big.Int))
-		} else {
-			return S256(value.Interface().(*big.Int))
-		}
-
+		return U256(value.Interface().(*big.Int))
 	}
 
 	return nil

accounts/abi/numbers_test.go

@@ -26,48 +26,28 @@ import (
 func TestNumberTypes(t *testing.T) {
 	ubytes := make([]byte, 32)
 	ubytes[31] = 1
-	sbytesmin := []byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}
 
 	unsigned := U256(big.NewInt(1))
 	if !bytes.Equal(unsigned, ubytes) {
 		t.Errorf("expected %x got %x", ubytes, unsigned)
 	}
-
-	signed := S256(big.NewInt(1))
-	if !bytes.Equal(signed, ubytes) {
-		t.Errorf("expected %x got %x", ubytes, unsigned)
-	}
-
-	signed = S256(big.NewInt(-1))
-	if !bytes.Equal(signed, sbytesmin) {
-		t.Errorf("expected %x got %x", ubytes, unsigned)
-	}
 }
 
 func TestPackNumber(t *testing.T) {
 	ubytes := make([]byte, 32)
 	ubytes[31] = 1
-	sbytesmin := []byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}
 	maxunsigned := []byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}
 
-	packed := packNum(reflect.ValueOf(1), IntTy)
-	if !bytes.Equal(packed, ubytes) {
-		t.Errorf("expected %x got %x", ubytes, packed)
-	}
-	packed = packNum(reflect.ValueOf(-1), IntTy)
-	if !bytes.Equal(packed, sbytesmin) {
-		t.Errorf("expected %x got %x", ubytes, packed)
-	}
-	packed = packNum(reflect.ValueOf(1), UintTy)
+	packed := packNum(reflect.ValueOf(1))
 	if !bytes.Equal(packed, ubytes) {
 		t.Errorf("expected %x got %x", ubytes, packed)
 	}
-	packed = packNum(reflect.ValueOf(-1), UintTy)
+	packed = packNum(reflect.ValueOf(-1))
 	if !bytes.Equal(packed, maxunsigned) {
 		t.Errorf("expected %x got %x", maxunsigned, packed)
 	}
 
-	packed = packNum(reflect.ValueOf("string"), UintTy)
+	packed = packNum(reflect.ValueOf("string"))
 	if packed != nil {
 		t.Errorf("expected 'string' to pack to nil. got %x instead", packed)
 	}

accounts/abi/packing.go

@@ -25,7 +25,7 @@ import (
 // packBytesSlice packs the given bytes as [L, V] as the canonical representation
 // bytes slice
 func packBytesSlice(bytes []byte, l int) []byte {
-	len := packNum(reflect.ValueOf(l), UintTy)
+	len := packNum(reflect.ValueOf(l))
 	return append(len, common.RightPadBytes(bytes, (l+31)/32*32)...)
 }
 
@@ -34,7 +34,7 @@ func packBytesSlice(bytes []byte, l int) []byte {
 func packElement(t Type, reflectValue reflect.Value) []byte {
 	switch t.T {
 	case IntTy, UintTy:
-		return packNum(reflectValue, t.T)
+		return packNum(reflectValue)
 	case StringTy:
 		return packBytesSlice([]byte(reflectValue.String()), reflectValue.Len())
 	case AddressTy: