fix: allow negative observations, error if bounds exceeded

pkg/solana/marshal_signed_int.go

@@ -0,0 +1,67 @@
+// code from: https://github.com/smartcontractkit/chainlink-terra/blob/develop/pkg/terra/marshal_signed_int.go
+// will eventually be removed and replaced with a generalized version from libocr
+
+package solana
+
+import (
+	"bytes"
+	"fmt"
+	"math/big"
+)
+
+var i = big.NewInt
+
+func bounds(numBytes uint) (*big.Int, *big.Int) {
+	max := i(0).Sub(i(0).Lsh(i(1), numBytes*8-1), i(1)) // 2**(numBytes*8-1)- 1
+	min := i(0).Sub(i(0).Neg(max), i(1))                // -2**(numBytes*8-1)
+	return min, max
+}
+
+// ToBigInt interprets bytes s as a big-endian signed integer
+// of size numBytes.
+func ToBigInt(s []byte, numBytes uint) (*big.Int, error) {
+	if uint(len(s)) != numBytes {
+		return nil, fmt.Errorf("invalid int length: expected %d got %d", numBytes, len(s))
+	}
+	val := (&big.Int{}).SetBytes(s)
+	numBits := numBytes * 8
+	_, max := bounds(numBytes)
+	negative := val.Cmp(max) > 0
+	if negative {
+		// Get the complement wrt to 2^numBits
+		maxUint := big.NewInt(1)
+		maxUint.Lsh(maxUint, numBits)
+		val.Sub(maxUint, val)
+		val.Neg(val)
+	}
+	return val, nil
+}
+
+// ToBytes converts *big.Int o into bytes as a big-endian signed
+// integer of size numBytes
+func ToBytes(o *big.Int, numBytes uint) ([]byte, error) {
+	min, max := bounds(numBytes)
+	if o.Cmp(max) > 0 || o.Cmp(min) < 0 {
+		return nil, fmt.Errorf("value won't fit in int%v: 0x%x", numBytes*8, o)
+	}
+	negative := o.Sign() < 0
+	val := (&big.Int{})
+	numBits := numBytes * 8
+	if negative {
+		// compute two's complement as 2**numBits - abs(o) = 2**numBits + o
+		val.SetInt64(1)
+		val.Lsh(val, numBits)
+		val.Add(val, o)
+	} else {
+		val.Set(o)
+	}
+	b := val.Bytes() // big-endian representation of abs(val)
+	if uint(len(b)) > numBytes {
+		return nil, fmt.Errorf("b must fit in %v bytes", numBytes)
+	}
+	b = bytes.Join([][]byte{bytes.Repeat([]byte{0}, int(numBytes)-len(b)), b}, []byte{})
+	if uint(len(b)) != numBytes {
+		return nil, fmt.Errorf("wrong length; there must be an error in the padding of b: %v", b)
+	}
+	return b, nil
+}

pkg/solana/marshal_signed_int_test.go

@@ -0,0 +1,186 @@
+package solana
+
+import (
+	"encoding/hex"
+	"math/big"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMarshalSignedInt(t *testing.T) {
+	var tt = []struct {
+		bytesVal  string
+		size      uint
+		expected  *big.Int
+		expectErr bool
+	}{
+		{
+			"ffffffffffffffff",
+			8,
+			big.NewInt(-1),
+			false,
+		},
+		{
+			"fffffffffffffffe",
+			8,
+			big.NewInt(-2),
+			false,
+		},
+		{
+			"0000000000000000",
+			8,
+			big.NewInt(0),
+			false,
+		},
+		{
+			"0000000000000001",
+			8,
+			big.NewInt(1),
+			false,
+		},
+		{
+			"0000000000000002",
+			8,
+			big.NewInt(2),
+			false,
+		},
+		{
+			"7fffffffffffffff",
+			8,
+			big.NewInt(9223372036854775807), // 2^63 - 1
+			false,
+		},
+		{
+			"00000000000000000000000000000000",
+			16,
+			big.NewInt(0),
+			false,
+		},
+		{
+			"00000000000000000000000000000001",
+			16,
+			big.NewInt(1),
+			false,
+		},
+		{
+			"00000000000000000000000000000002",
+			16,
+			big.NewInt(2),
+			false,
+		},
+		{
+			"7fffffffffffffffffffffffffffffff", // 2^127 - 1
+			16,
+			big.NewInt(0).Sub(big.NewInt(0).Lsh(big.NewInt(1), 127), big.NewInt(1)),
+			false,
+		},
+		{
+			"ffffffffffffffffffffffffffffffff",
+			16,
+			big.NewInt(-1),
+			false,
+		},
+		{
+			"fffffffffffffffffffffffffffffffe",
+			16,
+			big.NewInt(-2),
+			false,
+		},
+		{
+			"000000000000000000000000000000000000000000000000",
+			24,
+			big.NewInt(0),
+			false,
+		},
+		{
+			"000000000000000000000000000000000000000000000001",
+			24,
+			big.NewInt(1),
+			false,
+		},
+		{
+			"000000000000000000000000000000000000000000000002",
+			24,
+			big.NewInt(2),
+			false,
+		},
+		{
+			"ffffffffffffffffffffffffffffffffffffffffffffffff",
+			24,
+			big.NewInt(-1),
+			false,
+		},
+		{
+			"fffffffffffffffffffffffffffffffffffffffffffffffe",
+			24,
+			big.NewInt(-2),
+			false,
+		},
+	}
+	for _, tc := range tt {
+		tc := tc
+		b, err := hex.DecodeString(tc.bytesVal)
+		require.NoError(t, err)
+		i, err := ToBigInt(b, tc.size)
+		require.NoError(t, err)
+		assert.Equal(t, i.String(), tc.expected.String())
+
+		// Marshalling back should give us the same bytes
+		bAfter, err := ToBytes(i, tc.size)
+		require.NoError(t, err)
+		assert.Equal(t, tc.bytesVal, hex.EncodeToString(bAfter))
+	}
+
+	var tt2 = []struct {
+		o         *big.Int
+		numBytes  uint
+		expectErr bool
+	}{
+		{
+			big.NewInt(128),
+			1,
+			true,
+		},
+		{
+			big.NewInt(-129),
+			1,
+			true,
+		},
+		{
+			big.NewInt(-128),
+			1,
+			false,
+		},
+		{
+			big.NewInt(2147483648),
+			4,
+			true,
+		},
+		{
+			big.NewInt(2147483647),
+			4,
+			false,
+		},
+		{
+			big.NewInt(-2147483649),
+			4,
+			true,
+		},
+		{
+			big.NewInt(-2147483648),
+			4,
+			false,
+		},
+	}
+	for _, tc := range tt2 {
+		tc := tc
+		_, err := ToBytes(tc.o, tc.numBytes)
+		if tc.expectErr {
+			require.Error(t, err)
+		} else {
+			require.NoError(t, err)
+		}
+	}
+}

pkg/solana/report.go

@@ -7,6 +7,7 @@ import (
 	"math/big"
 	"sort"
 
+	"github.com/pkg/errors"
 	"github.com/smartcontractkit/libocr/offchainreporting2/chains/evmutil"
 	"github.com/smartcontractkit/libocr/offchainreporting2/reportingplugin/median"
 	"github.com/smartcontractkit/libocr/offchainreporting2/types"
@@ -62,11 +63,19 @@ func (c ReportCodec) BuildReport(oo []median.ParsedAttributedObservation) (types
 
 	report = append(report, observers[:]...)
 
-	mBytes := make([]byte, MedianLen)
-	report = append(report, median.FillBytes(mBytes)[:]...)
+	// TODO: replace with generalized function from libocr
+	medianBytes, err := ToBytes(median, uint(MedianLen))
+	if err != nil {
+		return nil, errors.Wrap(err, "error in ToBytes(median)")
+	}
+	report = append(report, medianBytes[:]...)
 
-	jBytes := make([]byte, JuelsLen)
-	report = append(report, juelsPerFeeCoin.FillBytes(jBytes)[:]...)
+	// TODO: replace with generalized function from libocr
+	juelsPerFeeCoinBytes, err := ToBytes(juelsPerFeeCoin, uint(JuelsLen))
+	if err != nil {
+		return nil, errors.Wrap(err, "error in ToBytes(juelsPerFeeCoin)")
+	}
+	report = append(report, juelsPerFeeCoinBytes[:]...)
 
 	return types.Report(report), nil
 }
@@ -81,7 +90,7 @@ func (c ReportCodec) MedianFromReport(report types.Report) (*big.Int, error) {
 	start := 4 + 1 + 32
 	end := start + int(MedianLen)
 	median := report[start:end]
-	return big.NewInt(0).SetBytes(median), nil
+	return ToBigInt(median, uint(MedianLen))
 }
 
 // Create report digest using SHA256 hash fn

pkg/solana/report_test.go

@@ -2,10 +2,12 @@ package solana
 
 import (
 	"encoding/binary"
+	"math"
 	"math/big"
 	"testing"
 	"time"
 
+	bin "github.com/gagliardetto/binary"
 	"github.com/smartcontractkit/libocr/commontypes"
 	"github.com/smartcontractkit/libocr/offchainreporting2/reportingplugin/median"
 	"github.com/smartcontractkit/libocr/offchainreporting2/types"
@@ -108,3 +110,63 @@ func TestHashReport(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, mockHash, h)
 }
+
+func TestNegativeMedianValue(t *testing.T) {
+	c := ReportCodec{}
+	oo := []median.ParsedAttributedObservation{
+		median.ParsedAttributedObservation{
+			Timestamp:       uint32(time.Now().Unix()),
+			Value:           big.NewInt(-2),
+			JuelsPerFeeCoin: big.NewInt(1),
+			Observer:        commontypes.OracleID(0),
+		},
+	}
+
+	// create report
+	report, err := c.BuildReport(oo)
+	assert.NoError(t, err)
+
+	// check report properly encoded negative number
+	index := 4 + 1 + 32
+	var medianFromRaw bin.Int128
+	medianBytes := make([]byte, MedianLen)
+	copy(medianBytes, report[index:index+int(MedianLen)])
+	// flip order: bin decoder parses from little endian
+	for i, j := 0, len(medianBytes)-1; i < j; i, j = i+1, j-1 {
+		medianBytes[i], medianBytes[j] = medianBytes[j], medianBytes[i]
+	}
+	bin.NewBinDecoder(medianBytes).Decode(&medianFromRaw)
+	assert.True(t, oo[0].Value.Cmp(medianFromRaw.BigInt()) == 0, "median observation in raw report does not match")
+
+	// check report can be parsed properly with a negative number
+	res, err := c.MedianFromReport(report)
+	assert.NoError(t, err)
+	assert.True(t, oo[0].Value.Cmp(res) == 0)
+}
+
+func TestReportHandleOverflow(t *testing.T) {
+	// too large observation should not cause panic
+	c := ReportCodec{}
+	oo := []median.ParsedAttributedObservation{
+		median.ParsedAttributedObservation{
+			Timestamp:       uint32(time.Now().Unix()),
+			Value:           big.NewInt(0).Lsh(big.NewInt(1), 127), // 1<<127
+			JuelsPerFeeCoin: big.NewInt(0),
+			Observer:        commontypes.OracleID(0),
+		},
+	}
+	_, err := c.BuildReport(oo)
+	assert.Error(t, err)
+
+	// too large juelsPerFeeCoin should not cause panic
+	oo = []median.ParsedAttributedObservation{
+		median.ParsedAttributedObservation{
+			Timestamp:       uint32(time.Now().Unix()),
+			Value:           big.NewInt(0),
+			JuelsPerFeeCoin: big.NewInt(0).Add(big.NewInt(math.MaxInt64), big.NewInt(1)),
+			Observer:        commontypes.OracleID(0),
+		},
+	}
+	_, err = c.BuildReport(oo)
+	assert.Error(t, err)
+}