Fix: ACL bugs (#1168)

* fix: bugs

* fix:  comments

Dockerfile

@@ -82,7 +82,7 @@ COPY --from=builder /app/build/bin/sentry /usr/local/bin/sentry
 COPY --from=builder /app/build/bin/state /usr/local/bin/state
 COPY --from=builder /app/build/bin/txpool /usr/local/bin/txpool
 COPY --from=builder /app/build/bin/verkle /usr/local/bin/verkle
-
+COPY --from=builder /app/build/bin/acl /usr/local/bin/acl
 
 
 EXPOSE 8545 \

Dockerfile.debian

@@ -74,6 +74,7 @@ COPY --from=builder /app/build/bin/sentry /usr/local/bin/sentry
 COPY --from=builder /app/build/bin/state /usr/local/bin/state
 COPY --from=builder /app/build/bin/txpool /usr/local/bin/txpool
 COPY --from=builder /app/build/bin/verkle /usr/local/bin/verkle
+COPY --from=builder /app/build/bin/acl /usr/local/bin/acl
 
 COPY --from=builder /go/pkg/mod /go/pkg/mod
 

zk/txpool/policy.go

@@ -53,14 +53,23 @@ func containsPolicy(policies []byte, policy Policy) bool {
 	return bytes.Contains(policies, policy.ToByteArray())
 }
 
-// CheckPolicy checks if the given address has the given policy for the online ACL mode
-func CheckPolicy(ctx context.Context, aclDB kv.RwDB, addr common.Address, policy Policy) (bool, error) {
+// DoesAccountHavePolicy checks if the given account has the given policy for the online ACL mode
+func DoesAccountHavePolicy(ctx context.Context, aclDB kv.RwDB, addr common.Address, policy Policy) (bool, error) {
+	hasPolicy, _, err := checkIfAccountHasPolicy(ctx, aclDB, addr, policy)
+	return hasPolicy, err
+}
+
+func checkIfAccountHasPolicy(ctx context.Context, aclDB kv.RwDB, addr common.Address, policy Policy) (bool, ACLMode, error) {
 	if !IsSupportedPolicy(policy) {
-		return false, errUnknownPolicy
+		return false, DisabledMode, errUnknownPolicy
 	}
 
 	// Retrieve the mode configuration
-	var hasPolicy bool
+	var (
+		hasPolicy bool
+		mode      ACLMode = DisabledMode
+	)
+
 	err := aclDB.View(ctx, func(tx kv.Tx) error {
 		value, err := tx.GetOne(Config, []byte("mode"))
 		if err != nil {
@@ -72,7 +81,7 @@ func CheckPolicy(ctx context.Context, aclDB kv.RwDB, addr common.Address, policy
 			return nil
 		}
 
-		mode := string(value)
+		mode = ACLMode(value)
 
 		table := BlockList
 		if mode == AllowlistMode {
@@ -87,18 +96,18 @@ func CheckPolicy(ctx context.Context, aclDB kv.RwDB, addr common.Address, policy
 
 		policyBytes = value
 		if policyBytes != nil && containsPolicy(policyBytes, policy) {
-			// If address is in the whitelist and has the policy, return true
-			// If address is in the blacklist and has the policy, return false
+			// If address is in the allowlist and has the policy, return true
+			// If address is in the blocklist and has the policy, return false
 			hasPolicy = true
 		}
 
 		return nil
 	})
 	if err != nil {
-		return false, err
+		return false, mode, err
 	}
 
-	return hasPolicy, nil
+	return hasPolicy, mode, nil
 }
 
 // UpdatePolicies sets a policy for an address
@@ -251,7 +260,19 @@ func resolvePolicy(txn *types.TxSlot) Policy {
 	return SendTx
 }
 
-// create a method checkpolicy to check an address according to passed policy in the method
-func (p *TxPool) checkPolicy(ctx context.Context, addr common.Address, policy Policy) (bool, error) {
-	return CheckPolicy(ctx, p.aclDB, addr, policy)
+// isActionAllowed checks if the given action is allowed for the given address
+func (p *TxPool) isActionAllowed(ctx context.Context, addr common.Address, policy Policy) (bool, error) {
+	hasPolicy, mode, err := checkIfAccountHasPolicy(ctx, p.aclDB, addr, policy)
+	if err != nil {
+		return false, err
+	}
+
+	switch mode {
+	case BlocklistMode:
+		// If the mode is blocklist, and address has a certain policy, then invert the result
+		// because, for example, if it has sendTx policy, it means it is not allowed to sendTx
+		return !hasPolicy, nil
+	default:
+		return hasPolicy, nil
+	}
 }

zk/txpool/policy_test.go

@@ -135,7 +135,7 @@ func TestRemovePolicy(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policy is removed from the ACL
-		hasPolicy, err := CheckPolicy(ctx, db, addr, policy)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr, policy)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 	})
@@ -155,7 +155,7 @@ func TestRemovePolicy(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policy is still not present in the ACL
-		hasPolicy, err := CheckPolicy(ctx, db, addr, policy)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr, policy)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 	})
@@ -172,7 +172,7 @@ func TestRemovePolicy(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policy is still not present in the ACL
-		hasPolicy, err := CheckPolicy(ctx, db, addr, policy)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr, policy)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 	})
@@ -208,7 +208,7 @@ func TestAddPolicy(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policy exists in the ACL
-		hasPolicy, err := CheckPolicy(ctx, db, addr, policy)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr, policy)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 	})
@@ -228,7 +228,7 @@ func TestAddPolicy(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policy still exists in the ACL
-		hasPolicy, err := CheckPolicy(ctx, db, addr, policy)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr, policy)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 	})
@@ -279,15 +279,15 @@ func TestUpdatePolicies(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policies are added correctly
-		hasPolicy, err := CheckPolicy(ctx, db, addr1, SendTx)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr1, SendTx)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr1, Deploy)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr1, Deploy)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr2, SendTx)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr2, SendTx)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 	})
@@ -307,15 +307,15 @@ func TestUpdatePolicies(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policies are removed correctly
-		hasPolicy, err := CheckPolicy(ctx, db, addr1, SendTx)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr1, SendTx)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr1, Deploy)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr1, Deploy)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr2, SendTx)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr2, SendTx)
 		require.NoError(t, err)
 		require.True(t, hasPolicy)
 	})
@@ -335,15 +335,15 @@ func TestUpdatePolicies(t *testing.T) {
 		require.NoError(t, err)
 
 		// Check if the policies are removed correctly
-		hasPolicy, err := CheckPolicy(ctx, db, addr1, SendTx)
+		hasPolicy, err := DoesAccountHavePolicy(ctx, db, addr1, SendTx)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr1, Deploy)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr1, Deploy)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 
-		hasPolicy, err = CheckPolicy(ctx, db, addr2, SendTx)
+		hasPolicy, err = DoesAccountHavePolicy(ctx, db, addr2, SendTx)
 		require.NoError(t, err)
 		require.False(t, hasPolicy)
 	})
@@ -363,3 +363,81 @@ func TestUpdatePolicies(t *testing.T) {
 		require.ErrorIs(t, err, errUnsupportedACLType)
 	})
 }
+
+func TestIsActionAllowed(t *testing.T) {
+	db := newTestACLDB(t, "")
+	ctx := context.Background()
+
+	txPool := &TxPool{aclDB: db}
+
+	t.Run("isActionAllowed - BlocklistMode - Policy Exists", func(t *testing.T) {
+		SetMode(ctx, db, BlocklistMode)
+
+		// Create a test address and policy
+		addr := common.HexToAddress("0x1234567890abcdef")
+		policy := SendTx
+
+		// Add the policy to the ACL
+		require.NoError(t, AddPolicy(ctx, db, "blocklist", addr, policy))
+
+		// Check if the action is allowed
+		allowed, err := txPool.isActionAllowed(ctx, addr, policy)
+		require.NoError(t, err)
+		require.False(t, allowed) // In blocklist mode, having the policy means the action is not allowed
+	})
+
+	t.Run("isActionAllowed - BlocklistMode - Policy Does Not Exist", func(t *testing.T) {
+		SetMode(ctx, db, BlocklistMode)
+
+		// Create a test address and policy
+		addr := common.HexToAddress("0x1234567890abcdef")
+		policy := Deploy
+
+		// Check if the action is allowed
+		allowed, err := txPool.isActionAllowed(ctx, addr, policy)
+		require.NoError(t, err)
+		require.True(t, allowed) // In blocklist mode, not having the policy means the action is allowed
+	})
+
+	t.Run("isActionAllowed - AllowlistMode - Policy Exists", func(t *testing.T) {
+		SetMode(ctx, db, AllowlistMode)
+
+		// Create a test address and policy
+		addr := common.HexToAddress("0x1234567890abcdef")
+		policy := SendTx
+
+		// Add the policy to the ACL
+		require.NoError(t, AddPolicy(ctx, db, "allowlist", addr, policy))
+
+		// Check if the action is allowed
+		allowed, err := txPool.isActionAllowed(ctx, addr, policy)
+		require.NoError(t, err)
+		require.True(t, allowed) // In allowlist mode, having the policy means the action is allowed
+	})
+
+	t.Run("isActionAllowed - AllowlistMode - Policy Does Not Exist", func(t *testing.T) {
+		SetMode(ctx, db, AllowlistMode)
+
+		// Create a test address and policy
+		addr := common.HexToAddress("0x1234567890abcdef")
+		policy := Deploy
+
+		// Check if the action is allowed
+		allowed, err := txPool.isActionAllowed(ctx, addr, policy)
+		require.NoError(t, err)
+		require.False(t, allowed) // In allowlist mode, not having the policy means the action is not allowed
+	})
+
+	t.Run("isActionAllowed - DisabledMode", func(t *testing.T) {
+		SetMode(ctx, db, DisabledMode)
+
+		// Create a test address and policy
+		addr := common.HexToAddress("0x1234567890abcdef")
+		policy := SendTx
+
+		// Check if the action is allowed
+		allowed, err := txPool.isActionAllowed(ctx, addr, policy)
+		require.NoError(t, err)
+		require.True(t, allowed) // In disabled mode, all actions are allowed
+	})
+}

zk/txpool/pool.go

@@ -782,7 +782,7 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 	switch resolvePolicy(txn) {
 	case SendTx:
 		var allow bool
-		allow, err := p.checkPolicy(context.TODO(), from, SendTx)
+		allow, err := p.isActionAllowed(context.TODO(), from, SendTx)
 		if err != nil {
 			panic(err)
 		}
@@ -792,7 +792,7 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 	case Deploy:
 		var allow bool
 		// check that sender may deploy contracts
-		allow, err := p.checkPolicy(context.TODO(), from, Deploy)
+		allow, err := p.isActionAllowed(context.TODO(), from, Deploy)
 		if err != nil {
 			panic(err)
 		}