ACP-77: Refactor subnet auth verification (#3537)

vms/platformvm/txs/executor/staker_tx_verification.go

@@ -252,7 +252,7 @@ func verifyAddSubnetValidatorTx(
 		return err
 	}
 
-	baseTxCreds, err := verifyPoASubnetAuthorization(backend, chainState, sTx, tx.SubnetValidator.Subnet, tx.SubnetAuth)
+	baseTxCreds, err := verifyPoASubnetAuthorization(backend.Fx, chainState, sTx, tx.SubnetValidator.Subnet, tx.SubnetAuth)
 	if err != nil {
 		return err
 	}
@@ -332,7 +332,7 @@ func verifyRemoveSubnetValidatorTx(
 		return vdr, isCurrentValidator, nil
 	}
 
-	baseTxCreds, err := verifySubnetAuthorization(backend, chainState, sTx, tx.Subnet, tx.SubnetAuth)
+	baseTxCreds, err := verifySubnetAuthorization(backend.Fx, chainState, sTx, tx.Subnet, tx.SubnetAuth)
 	if err != nil {
 		return nil, false, err
 	}
@@ -781,7 +781,7 @@ func verifyTransferSubnetOwnershipTx(
 		return nil
 	}
 
-	baseTxCreds, err := verifySubnetAuthorization(backend, chainState, sTx, tx.Subnet, tx.SubnetAuth)
+	baseTxCreds, err := verifySubnetAuthorization(backend.Fx, chainState, sTx, tx.Subnet, tx.SubnetAuth)
 	if err != nil {
 		return err
 	}

vms/platformvm/txs/executor/standard_tx_executor.go

@@ -197,7 +197,7 @@ func (e *standardTxExecutor) CreateChainTx(tx *txs.CreateChainTx) error {
 		return err
 	}
 
-	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend, e.state, e.tx, tx.SubnetID, tx.SubnetAuth)
+	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend.Fx, e.state, e.tx, tx.SubnetID, tx.SubnetAuth)
 	if err != nil {
 		return err
 	}
@@ -510,7 +510,7 @@ func (e *standardTxExecutor) TransformSubnetTx(tx *txs.TransformSubnetTx) error
 		return errMaxStakeDurationTooLarge
 	}
 
-	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend, e.state, e.tx, tx.Subnet, tx.SubnetAuth)
+	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend.Fx, e.state, e.tx, tx.Subnet, tx.SubnetAuth)
 	if err != nil {
 		return err
 	}
@@ -689,7 +689,7 @@ func (e *standardTxExecutor) ConvertSubnetTx(tx *txs.ConvertSubnetTx) error {
 		return err
 	}
 
-	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend, e.state, e.tx, tx.Subnet, tx.SubnetAuth)
+	baseTxCreds, err := verifyPoASubnetAuthorization(e.backend.Fx, e.state, e.tx, tx.Subnet, tx.SubnetAuth)
 	if err != nil {
 		return err
 	}

vms/platformvm/txs/executor/standard_tx_executor_test.go

@@ -1873,14 +1873,13 @@ func TestStandardExecutorRemoveSubnetValidatorTx(t *testing.T) {
 			expectedErr: ErrRemovePermissionlessValidator,
 		},
 		{
-			name: "tx has no credentials",
+			name: "can't find subnet",
 			newExecutor: func(ctrl *gomock.Controller) (*txs.RemoveSubnetValidatorTx, *standardTxExecutor) {
 				env := newValidRemoveSubnetValidatorTxVerifyEnv(t, ctrl)
-				// Remove credentials
-				env.tx.Creds = nil
 				env.state = state.NewMockDiff(ctrl)
 				env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
 				env.state.EXPECT().GetCurrentValidator(env.unsignedTx.Subnet, env.unsignedTx.NodeID).Return(env.staker, nil)
+				env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(nil, database.ErrNotFound)
 
 				cfg := &config.Internal{
 					UpgradeConfig: upgradetest.GetConfigWithUpgradeTime(upgradetest.Durango, env.latestForkTime),
@@ -1900,16 +1899,19 @@ func TestStandardExecutorRemoveSubnetValidatorTx(t *testing.T) {
 				}
 				return env.unsignedTx, e
 			},
-			expectedErr: errWrongNumberOfCredentials,
+			expectedErr: database.ErrNotFound,
 		},
 		{
-			name: "can't find subnet",
+			name: "tx has no credentials",
 			newExecutor: func(ctrl *gomock.Controller) (*txs.RemoveSubnetValidatorTx, *standardTxExecutor) {
 				env := newValidRemoveSubnetValidatorTxVerifyEnv(t, ctrl)
+				// Remove credentials
+				env.tx.Creds = nil
 				env.state = state.NewMockDiff(ctrl)
 				env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
 				env.state.EXPECT().GetCurrentValidator(env.unsignedTx.Subnet, env.unsignedTx.NodeID).Return(env.staker, nil)
-				env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(nil, database.ErrNotFound)
+				subnetOwner := fxmock.NewOwner(ctrl)
+				env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(subnetOwner, nil).AnyTimes()
 
 				cfg := &config.Internal{
 					UpgradeConfig: upgradetest.GetConfigWithUpgradeTime(upgradetest.Durango, env.latestForkTime),
@@ -1929,7 +1931,7 @@ func TestStandardExecutorRemoveSubnetValidatorTx(t *testing.T) {
 				}
 				return env.unsignedTx, e
 			},
-			expectedErr: database.ErrNotFound,
+			expectedErr: errWrongNumberOfCredentials,
 		},
 		{
 			name: "no permission to remove validator",
@@ -2190,7 +2192,9 @@ func TestStandardExecutorTransformSubnetTx(t *testing.T) {
 				// Remove credentials
 				env.tx.Creds = nil
 				env.state = state.NewMockDiff(ctrl)
+				subnetOwner := fxmock.NewOwner(ctrl)
 				env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
+				env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(subnetOwner, nil).AnyTimes()
 
 				cfg := &config.Internal{
 					UpgradeConfig:    upgradetest.GetConfigWithUpgradeTime(upgradetest.Durango, env.latestForkTime),

vms/platformvm/txs/executor/subnet_tx_verification.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/vms/components/verify"
+	"github.com/ava-labs/avalanchego/vms/platformvm/fx"
 	"github.com/ava-labs/avalanchego/vms/platformvm/state"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs"
 )
@@ -24,13 +25,13 @@ var (
 // subnet. This is an extension of [verifySubnetAuthorization] that additionally
 // verifies that the subnet being modified is currently a PoA subnet.
 func verifyPoASubnetAuthorization(
-	backend *Backend,
+	fx fx.Fx,
 	chainState state.Chain,
 	sTx *txs.Tx,
 	subnetID ids.ID,
 	subnetAuth verify.Verifiable,
 ) ([]verify.Verifiable, error) {
-	creds, err := verifySubnetAuthorization(backend, chainState, sTx, subnetID, subnetAuth)
+	creds, err := verifySubnetAuthorization(fx, chainState, sTx, subnetID, subnetAuth)
 	if err != nil {
 		return nil, err
 	}
@@ -55,32 +56,45 @@ func verifyPoASubnetAuthorization(
 }
 
 // verifySubnetAuthorization carries out the validation for modifying a subnet.
-// The last credential in [sTx.Creds] is used as the subnet authorization.
+// The last credential in [tx.Creds] is used as the subnet authorization.
 // Returns the remaining tx credentials that should be used to authorize the
 // other operations in the tx.
 func verifySubnetAuthorization(
-	backend *Backend,
+	fx fx.Fx,
 	chainState state.Chain,
-	sTx *txs.Tx,
+	tx *txs.Tx,
 	subnetID ids.ID,
 	subnetAuth verify.Verifiable,
 ) ([]verify.Verifiable, error) {
-	if len(sTx.Creds) == 0 {
-		// Ensure there is at least one credential for the subnet authorization
-		return nil, errWrongNumberOfCredentials
-	}
-
-	baseTxCredsLen := len(sTx.Creds) - 1
-	subnetCred := sTx.Creds[baseTxCredsLen]
-
 	subnetOwner, err := chainState.GetSubnetOwner(subnetID)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := backend.Fx.VerifyPermission(sTx.Unsigned, subnetAuth, subnetCred, subnetOwner); err != nil {
+	return verifyAuthorization(fx, tx, subnetOwner, subnetAuth)
+}
+
+// verifyAuthorization carries out the validation of an auth. The last
+// credential in [tx.Creds] is used as the authorization.
+// Returns the remaining tx credentials that should be used to authorize the
+// other operations in the tx.
+func verifyAuthorization(
+	fx fx.Fx,
+	tx *txs.Tx,
+	owner fx.Owner,
+	auth verify.Verifiable,
+) ([]verify.Verifiable, error) {
+	if len(tx.Creds) == 0 {
+		// Ensure there is at least one credential for the subnet authorization
+		return nil, errWrongNumberOfCredentials
+	}
+
+	baseTxCredsLen := len(tx.Creds) - 1
+	authCred := tx.Creds[baseTxCredsLen]
+
+	if err := fx.VerifyPermission(tx.Unsigned, auth, authCred, owner); err != nil {
 		return nil, fmt.Errorf("%w: %w", errUnauthorizedSubnetModification, err)
 	}
 
-	return sTx.Creds[:baseTxCredsLen], nil
+	return tx.Creds[:baseTxCredsLen], nil
 }