verify single predicate (#960)

* verify single predicate * simplify returned error * track indexes for predicates * don't short cirtcuit predicate err * Update predicate_check.go Signed-off-by: Ceyhun Onur <[email protected]> * nits for single-predicate-verify (#1012) * Update x/warp/predicate_test.go Co-authored-by: aaronbuchwald <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]> * Update x/warp/predicate_test.go Co-authored-by: aaronbuchwald <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]> * review fixes * use bitset constructor * change return type to err in verifypredicate --------- Signed-off-by: Ceyhun Onur <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]> Co-authored-by: marun <[email protected]> Co-authored-by: Darioush Jalali <[email protected]> Co-authored-by: aaronbuchwald <[email protected]>
ava-labs · Dec 5, 2023 · 58eb7ef · 58eb7ef
1 parent 7a6bcad
commit 58eb7ef
Show file tree

Hide file tree

Showing 7 changed files with 270 additions and 163 deletions.
diff --git a/core/predicate_check.go b/core/predicate_check.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/ava-labs/avalanchego/utils/set"
 	"github.com/ava-labs/subnet-evm/core/types"
 	"github.com/ava-labs/subnet-evm/params"
 	"github.com/ava-labs/subnet-evm/precompile/precompileconfig"
@@ -31,7 +32,7 @@ func CheckPredicates(rules params.Rules, predicateContext *precompileconfig.Pred
 
 	predicateResults := make(map[common.Address][]byte)
 	// Short circuit early if there are no precompile predicates to verify
-	if len(rules.Predicaters) == 0 {
+	if !rules.PredicatersExist() {
 		return predicateResults, nil
 	}
 
@@ -52,10 +53,15 @@ func CheckPredicates(rules params.Rules, predicateContext *precompileconfig.Pred
 		// Since [address] is only added to [predicateArguments] when there's a valid predicate in the ruleset
 		// there's no need to check if the predicate exists here.
 		predicaterContract := rules.Predicaters[address]
-		res := predicaterContract.VerifyPredicate(predicateContext, predicates)
+		bitset := set.NewBits()
+		for i, predicate := range predicates {
+			if err := predicaterContract.VerifyPredicate(predicateContext, predicate); err != nil {
+				bitset.Add(i)
+			}
+		}
+		res := bitset.Bytes()
 		log.Debug("predicate verify", "tx", tx.Hash(), "address", address, "res", res)
 		predicateResults[address] = res
 	}
-
 	return predicateResults, nil
 }
diff --git a/core/predicate_check_test.go b/core/predicate_check_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
+	"github.com/ava-labs/avalanchego/utils/set"
 	"github.com/ava-labs/subnet-evm/core/types"
 	"github.com/ava-labs/subnet-evm/params"
 	"github.com/ava-labs/subnet-evm/precompile/precompileconfig"
@@ -31,8 +32,6 @@ func TestCheckPredicate(t *testing.T) {
 	addr2 := common.HexToAddress("0xbb")
 	addr3 := common.HexToAddress("0xcc")
 	addr4 := common.HexToAddress("0xdd")
-	predicateResultBytes1 := []byte{1, 2, 3}
-	predicateResultBytes2 := []byte{3, 2, 1}
 	predicateContext := &precompileconfig.PredicateContext{
 		ProposerVMBlockCtx: &block.Context{
 			PChainHeight: 10,
@@ -142,7 +141,7 @@ func TestCheckPredicate(t *testing.T) {
 				predicater := precompileconfig.NewMockPredicater(gomock.NewController(t))
 				arg := common.Hash{1}
 				predicater.EXPECT().PredicateGas(arg[:]).Return(uint64(0), nil).Times(2)
-				predicater.EXPECT().VerifyPredicate(gomock.Any(), [][]byte{arg[:]}).Return(predicateResultBytes1)
+				predicater.EXPECT().VerifyPredicate(gomock.Any(), arg[:]).Return(nil)
 				return map[common.Address]precompileconfig.Predicater{
 					addr1: predicater,
 				}
@@ -156,7 +155,7 @@ func TestCheckPredicate(t *testing.T) {
 				},
 			}),
 			expectedRes: map[common.Address][]byte{
-				addr1: predicateResultBytes1,
+				addr1: {}, // valid bytes
 			},
 			expectedErr: nil,
 		},
@@ -188,7 +187,7 @@ func TestCheckPredicate(t *testing.T) {
 				predicater := precompileconfig.NewMockPredicater(gomock.NewController(t))
 				arg := common.Hash{1}
 				predicater.EXPECT().PredicateGas(arg[:]).Return(uint64(0), nil).Times(2)
-				predicater.EXPECT().VerifyPredicate(gomock.Any(), [][]byte{arg[:]}).Return(predicateResultBytes1)
+				predicater.EXPECT().VerifyPredicate(gomock.Any(), arg[:]).Return(nil)
 				return map[common.Address]precompileconfig.Predicater{
 					addr1: predicater,
 					addr2: predicater,
@@ -203,7 +202,7 @@ func TestCheckPredicate(t *testing.T) {
 				},
 			}),
 			expectedRes: map[common.Address][]byte{
-				addr1: predicateResultBytes1,
+				addr1: {}, // valid bytes
 			},
 			expectedErr: nil,
 		},
@@ -215,11 +214,11 @@ func TestCheckPredicate(t *testing.T) {
 				predicate1 := precompileconfig.NewMockPredicater(ctrl)
 				arg1 := common.Hash{1}
 				predicate1.EXPECT().PredicateGas(arg1[:]).Return(uint64(0), nil).Times(2)
-				predicate1.EXPECT().VerifyPredicate(gomock.Any(), [][]byte{arg1[:]}).Return(predicateResultBytes1)
+				predicate1.EXPECT().VerifyPredicate(gomock.Any(), arg1[:]).Return(nil)
 				predicate2 := precompileconfig.NewMockPredicater(ctrl)
 				arg2 := common.Hash{2}
 				predicate2.EXPECT().PredicateGas(arg2[:]).Return(uint64(0), nil).Times(2)
-				predicate2.EXPECT().VerifyPredicate(gomock.Any(), [][]byte{arg2[:]}).Return(predicateResultBytes2)
+				predicate2.EXPECT().VerifyPredicate(gomock.Any(), arg2[:]).Return(testErr)
 				return map[common.Address]precompileconfig.Predicater{
 					addr1: predicate1,
 					addr2: predicate2,
@@ -240,8 +239,8 @@ func TestCheckPredicate(t *testing.T) {
 				},
 			}),
 			expectedRes: map[common.Address][]byte{
-				addr1: predicateResultBytes1,
-				addr2: predicateResultBytes2,
+				addr1: {},  // valid bytes
+				addr2: {1}, // invalid bytes
 			},
 			expectedErr: nil,
 		},
@@ -322,3 +321,141 @@ func TestCheckPredicate(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckPredicatesOutput(t *testing.T) {
+	testErr := errors.New("test error")
+	addr1 := common.HexToAddress("0xaa")
+	addr2 := common.HexToAddress("0xbb")
+	validHash := common.Hash{1}
+	invalidHash := common.Hash{2}
+	predicateContext := &precompileconfig.PredicateContext{
+		ProposerVMBlockCtx: &block.Context{
+			PChainHeight: 10,
+		},
+	}
+	type testTuple struct {
+		address          common.Address
+		isValidPredicate bool
+	}
+	type resultTest struct {
+		name        string
+		expectedRes map[common.Address][]byte
+		testTuple   []testTuple
+	}
+	tests := []resultTest{
+		{name: "no predicates", expectedRes: map[common.Address][]byte{}},
+		{
+			name: "one address one predicate",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: true},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits().Bytes()},
+		},
+		{
+			name: "one address one invalid predicate",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: false},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(0).Bytes()},
+		},
+		{
+			name: "one address two invalid predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(0, 1).Bytes()},
+		},
+		{
+			name: "one address two mixed predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: true},
+				{address: addr1, isValidPredicate: false},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(1).Bytes()},
+		},
+		{
+			name: "one address mixed predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: true},
+				{address: addr1, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+				{address: addr1, isValidPredicate: true},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(1, 2).Bytes()},
+		},
+		{
+			name: "two addresses mixed predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: true},
+				{address: addr2, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+				{address: addr2, isValidPredicate: true},
+				{address: addr2, isValidPredicate: true},
+				{address: addr2, isValidPredicate: false},
+				{address: addr2, isValidPredicate: true},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(1, 2).Bytes(), addr2: set.NewBits(0, 3).Bytes()},
+		},
+		{
+			name: "two addresses all valid predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: true},
+				{address: addr2, isValidPredicate: true},
+				{address: addr1, isValidPredicate: true},
+				{address: addr1, isValidPredicate: true},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits().Bytes(), addr2: set.NewBits().Bytes()},
+		},
+		{
+			name: "two addresses all invalid predicates",
+			testTuple: []testTuple{
+				{address: addr1, isValidPredicate: false},
+				{address: addr2, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+				{address: addr1, isValidPredicate: false},
+			},
+			expectedRes: map[common.Address][]byte{addr1: set.NewBits(0, 1, 2).Bytes(), addr2: set.NewBits(0).Bytes()},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			require := require.New(t)
+			// Create the rules from TestChainConfig and update the predicates based on the test params
+			rules := params.TestChainConfig.AvalancheRules(common.Big0, 0)
+			predicater := precompileconfig.NewMockPredicater(gomock.NewController(t))
+			predicater.EXPECT().PredicateGas(gomock.Any()).Return(uint64(0), nil).Times(len(test.testTuple))
+
+			var txAccessList types.AccessList
+			for _, tuple := range test.testTuple {
+				var predicateHash common.Hash
+				if tuple.isValidPredicate {
+					predicateHash = validHash
+					predicater.EXPECT().VerifyPredicate(gomock.Any(), validHash[:]).Return(nil)
+				} else {
+					predicateHash = invalidHash
+					predicater.EXPECT().VerifyPredicate(gomock.Any(), invalidHash[:]).Return(testErr)
+				}
+				txAccessList = append(txAccessList, types.AccessTuple{
+					Address: tuple.address,
+					StorageKeys: []common.Hash{
+						predicateHash,
+					},
+				})
+			}
+
+			rules.Predicaters[addr1] = predicater
+			rules.Predicaters[addr2] = predicater
+
+			// Specify only the access list, since this test should not depend on any other values
+			tx := types.NewTx(&types.DynamicFeeTx{
+				AccessList: txAccessList,
+				Gas:        53000,
+			})
+			predicateRes, err := CheckPredicates(rules, predicateContext, tx)
+			require.NoError(err)
+			require.Equal(test.expectedRes, predicateRes)
+		})
+	}
+}
diff --git a/precompile/precompileconfig/config.go b/precompile/precompileconfig/config.go
@@ -48,8 +48,8 @@ type PredicateContext struct {
 // will not maintain backwards compatibility of this interface and your code should not
 // rely on this. Designed for use only by precompiles that ship with subnet-evm.
 type Predicater interface {
-	PredicateGas(storageSlots []byte) (uint64, error)
-	VerifyPredicate(predicateContext *PredicateContext, predicates [][]byte) []byte
+	PredicateGas(predicateBytes []byte) (uint64, error)
+	VerifyPredicate(predicateContext *PredicateContext, predicateBytes []byte) error
 }
 
 // SharedMemoryWriter defines an interface to allow a precompile's Accepter to write operations

diff --git a/precompile/precompileconfig/mocks.go b/precompile/precompileconfig/mocks.go
diff --git a/precompile/testutils/test_predicate.go b/precompile/testutils/test_predicate.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/ava-labs/subnet-evm/precompile/precompileconfig"
-	cmath "github.com/ethereum/go-ethereum/common/math"
 	"github.com/stretchr/testify/require"
 )
 
@@ -18,48 +17,27 @@ type PredicateTest struct {
 
 	PredicateContext *precompileconfig.PredicateContext
 
-	StorageSlots [][]byte
-	Gas          uint64
-	GasErr       error
-	PredicateRes []byte
+	PredicateBytes []byte
+	Gas            uint64
+	GasErr         error
+	ExpectedErr    error
 }
 
 func (test PredicateTest) Run(t testing.TB) {
 	t.Helper()
 	require := require.New(t)
+	predicate := test.Config.(precompileconfig.Predicater)
 
-	var (
-		gas          uint64
-		gasErr       error
-		predicateRes []byte
-		predicate    = test.Config.(precompileconfig.Predicater)
-	)
-
-	for _, predicateBytes := range test.StorageSlots {
-		predicateGas, predicateGasErr := predicate.PredicateGas(predicateBytes)
-		if predicateGasErr != nil {
-			gasErr = predicateGasErr
-			break
-		}
-		updatedGas, overflow := cmath.SafeAdd(gas, predicateGas)
-		if overflow {
-			panic("predicate gas should not overflow")
-		}
-		gas = updatedGas
-	}
-
+	predicateGas, predicateGasErr := predicate.PredicateGas(test.PredicateBytes)
+	require.ErrorIs(predicateGasErr, test.GasErr)
 	if test.GasErr != nil {
-		// If PredicateGas returns an error, the predicate fails verification and we will
-		// never call VerifyPredicate.
-		require.ErrorIs(gasErr, test.GasErr)
 		return
-	} else {
-		require.NoError(gasErr)
 	}
-	require.Equal(test.Gas, gas)
 
-	predicateRes = predicate.VerifyPredicate(test.PredicateContext, test.StorageSlots)
-	require.Equal(test.PredicateRes, predicateRes)
+	require.Equal(test.Gas, predicateGas)
+
+	predicateRes := predicate.VerifyPredicate(test.PredicateContext, test.PredicateBytes)
+	require.ErrorIs(predicateRes, test.ExpectedErr)
 }
 
 func RunPredicateTests(t *testing.T, predicateTests map[string]PredicateTest) {