Skip to content

Commit ac87871

Browse files
dhrubabasudhrubabasu
authored
Restrict Owner usage after a Subnet manager is set (#3147)
Signed-off-by: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
1 parent 4b4ec8a commit ac87871

File tree

4 files changed

+157
-0
lines changed

4 files changed

+157
-0
lines changed

‎vms/platformvm/txs/executor/create_chain_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,3 +253,40 @@ func TestCreateChainTxAP3FeeChange(t *testing.T) {
253253
})
254254
}
255255
}
256+
257+
func TestEtnaCreateChainTxInvalidWithManagedSubnet(t *testing.T) {
258+
require := require.New(t)
259+
env := newEnvironment(t, upgradetest.Etna)
260+
env.ctx.Lock.Lock()
261+
defer env.ctx.Lock.Unlock()
262+
263+
builder, signer := env.factory.NewWallet(testSubnet1ControlKeys[0], testSubnet1ControlKeys[1])
264+
utx, err := builder.NewCreateChainTx(
265+
testSubnet1.ID(),
266+
nil,
267+
constants.AVMID,
268+
nil,
269+
"chain name",
270+
)
271+
require.NoError(err)
272+
tx, err := walletsigner.SignUnsigned(context.Background(), signer, utx)
273+
require.NoError(err)
274+
275+
stateDiff, err := state.NewDiff(lastAcceptedID, env)
276+
require.NoError(err)
277+
278+
builderDiff, err := state.NewDiffOn(stateDiff)
279+
require.NoError(err)
280+
281+
stateDiff.SetSubnetManager(testSubnet1.ID(), ids.GenerateTestID(), []byte{'a', 'd', 'd', 'r', 'e', 's', 's'})
282+
283+
feeCalculator := state.PickFeeCalculator(env.config, builderDiff)
284+
executor := StandardTxExecutor{
285+
Backend: &env.backend,
286+
FeeCalculator: feeCalculator,
287+
State: stateDiff,
288+
Tx: tx,
289+
}
290+
err = tx.Unsigned.Visit(&executor)
291+
require.ErrorIs(err, errIsImmutable)
292+
}

‎vms/platformvm/txs/executor/staker_tx_verification.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ var (
4141
ErrDurangoUpgradeNotActive = errors.New("attempting to use a Durango-upgrade feature prior to activation")
4242
ErrAddValidatorTxPostDurango = errors.New("AddValidatorTx is not permitted post-Durango")
4343
ErrAddDelegatorTxPostDurango = errors.New("AddDelegatorTx is not permitted post-Durango")
44+
ErrRemoveValidatorManagedSubnet = errors.New("RemoveSubnetValidatorTx cannot be used to remove a validator from a Subnet with a manager")
4445
)
4546

4647
// verifySubnetValidatorPrimaryNetworkRequirements verifies the primary
@@ -306,6 +307,16 @@ func verifyRemoveSubnetValidatorTx(
306307
return nil, false, err
307308
}
308309

310+
if backend.Config.UpgradeConfig.IsEtnaActivated(currentTimestamp) {
311+
_, _, err := chainState.GetSubnetManager(tx.Subnet)
312+
if err == nil {
313+
return nil, false, fmt.Errorf("%w: %q", ErrRemoveValidatorManagedSubnet, tx.Subnet)
314+
}
315+
if err != database.ErrNotFound {
316+
return nil, false, err
317+
}
318+
}
319+
309320
isCurrentValidator := true
310321
vdr, err := chainState.GetCurrentValidator(tx.Subnet, tx.NodeID)
311322
if err == database.ErrNotFound {

‎vms/platformvm/txs/executor/standard_tx_executor_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,44 @@ func TestApricotStandardTxExecutorAddSubnetValidator(t *testing.T) {
891891
}
892892
}
893893

894+
func TestEtnaStandardTxExecutorAddSubnetValidator(t *testing.T) {
895+
require := require.New(t)
896+
env := newEnvironment(t, upgradetest.Etna)
897+
env.ctx.Lock.Lock()
898+
defer env.ctx.Lock.Unlock()
899+
900+
nodeID := genesisNodeIDs[0]
901+
902+
builder, signer := env.factory.NewWallet(testSubnet1ControlKeys[0], testSubnet1ControlKeys[1])
903+
utx, err := builder.NewAddSubnetValidatorTx(
904+
&txs.SubnetValidator{
905+
Validator: txs.Validator{
906+
NodeID: nodeID,
907+
Start: uint64(defaultValidateStartTime.Unix() + 1),
908+
End: uint64(defaultValidateEndTime.Unix()),
909+
Wght: defaultWeight,
910+
},
911+
Subnet: testSubnet1.ID(),
912+
},
913+
)
914+
require.NoError(err)
915+
tx, err := walletsigner.SignUnsigned(context.Background(), signer, utx)
916+
require.NoError(err)
917+
918+
onAcceptState, err := state.NewDiff(lastAcceptedID, env)
919+
require.NoError(err)
920+
921+
onAcceptState.SetSubnetManager(testSubnet1.ID(), ids.GenerateTestID(), []byte{'a', 'd', 'd', 'r', 'e', 's', 's'})
922+
923+
executor := StandardTxExecutor{
924+
Backend: &env.backend,
925+
State: onAcceptState,
926+
Tx: tx,
927+
}
928+
err = tx.Unsigned.Visit(&executor)
929+
require.ErrorIs(err, errIsImmutable)
930+
}
931+
894932
func TestBanffStandardTxExecutorAddValidator(t *testing.T) {
895933
require := require.New(t)
896934
env := newEnvironment(t, upgradetest.Banff)
@@ -1984,6 +2022,32 @@ func TestStandardExecutorRemoveSubnetValidatorTx(t *testing.T) {
19842022
},
19852023
expectedErr: ErrFlowCheckFailed,
19862024
},
2025+
{
2026+
name: "attempted to remove subnet validator after subnet manager is set",
2027+
newExecutor: func(ctrl *gomock.Controller) (*txs.RemoveSubnetValidatorTx, *StandardTxExecutor) {
2028+
env := newValidRemoveSubnetValidatorTxVerifyEnv(t, ctrl)
2029+
env.state.EXPECT().GetSubnetManager(env.unsignedTx.Subnet).Return(ids.GenerateTestID(), []byte{'a', 'd', 'd', 'r', 'e', 's', 's'}, nil).AnyTimes()
2030+
env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
2031+
2032+
cfg := &config.Config{
2033+
UpgradeConfig: upgradetest.GetConfigWithUpgradeTime(upgradetest.Etna, env.latestForkTime),
2034+
}
2035+
e := &StandardTxExecutor{
2036+
Backend: &Backend{
2037+
Config: cfg,
2038+
Bootstrapped: &utils.Atomic[bool]{},
2039+
Fx: env.fx,
2040+
FlowChecker: env.flowChecker,
2041+
Ctx: &snow.Context{},
2042+
},
2043+
Tx: env.tx,
2044+
State: env.state,
2045+
}
2046+
e.Bootstrapped.Set(true)
2047+
return env.unsignedTx, e
2048+
},
2049+
expectedErr: ErrRemoveValidatorManagedSubnet,
2050+
},
19872051
}
19882052

19892053
for _, tt := range tests {
@@ -2213,6 +2277,7 @@ func TestStandardExecutorTransformSubnetTx(t *testing.T) {
22132277
subnetOwner := fx.NewMockOwner(ctrl)
22142278
env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
22152279
env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(subnetOwner, nil)
2280+
env.state.EXPECT().GetSubnetManager(env.unsignedTx.Subnet).Return(ids.Empty, nil, database.ErrNotFound).Times(1)
22162281
env.state.EXPECT().GetSubnetTransformation(env.unsignedTx.Subnet).Return(nil, database.ErrNotFound).Times(1)
22172282
env.fx.EXPECT().VerifyPermission(gomock.Any(), env.unsignedTx.SubnetAuth, env.tx.Creds[len(env.tx.Creds)-1], subnetOwner).Return(nil)
22182283
env.flowChecker.EXPECT().VerifySpend(
@@ -2242,6 +2307,41 @@ func TestStandardExecutorTransformSubnetTx(t *testing.T) {
22422307
},
22432308
err: ErrFlowCheckFailed,
22442309
},
2310+
{
2311+
name: "invalid if subnet manager is set",
2312+
newExecutor: func(ctrl *gomock.Controller) (*txs.TransformSubnetTx, *StandardTxExecutor) {
2313+
env := newValidTransformSubnetTxVerifyEnv(t, ctrl)
2314+
2315+
// Set dependency expectations.
2316+
subnetOwner := fx.NewMockOwner(ctrl)
2317+
env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
2318+
env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(subnetOwner, nil).Times(1)
2319+
env.state.EXPECT().GetSubnetManager(env.unsignedTx.Subnet).Return(ids.GenerateTestID(), make([]byte, 20), nil)
2320+
env.state.EXPECT().GetSubnetTransformation(env.unsignedTx.Subnet).Return(nil, database.ErrNotFound).Times(1)
2321+
env.fx.EXPECT().VerifyPermission(env.unsignedTx, env.unsignedTx.SubnetAuth, env.tx.Creds[len(env.tx.Creds)-1], subnetOwner).Return(nil).Times(1)
2322+
2323+
cfg := &config.Config{
2324+
UpgradeConfig: upgradetest.GetConfigWithUpgradeTime(upgradetest.Durango, env.latestForkTime),
2325+
MaxStakeDuration: math.MaxInt64,
2326+
}
2327+
feeCalculator := state.PickFeeCalculator(cfg, env.state)
2328+
e := &StandardTxExecutor{
2329+
Backend: &Backend{
2330+
Config: cfg,
2331+
Bootstrapped: &utils.Atomic[bool]{},
2332+
Fx: env.fx,
2333+
FlowChecker: env.flowChecker,
2334+
Ctx: &snow.Context{},
2335+
},
2336+
FeeCalculator: feeCalculator,
2337+
Tx: env.tx,
2338+
State: env.state,
2339+
}
2340+
e.Bootstrapped.Set(true)
2341+
return env.unsignedTx, e
2342+
},
2343+
err: errIsImmutable,
2344+
},
22452345
{
22462346
name: "valid tx",
22472347
newExecutor: func(ctrl *gomock.Controller) (*txs.TransformSubnetTx, *StandardTxExecutor) {
@@ -2251,6 +2351,7 @@ func TestStandardExecutorTransformSubnetTx(t *testing.T) {
22512351
subnetOwner := fx.NewMockOwner(ctrl)
22522352
env.state.EXPECT().GetTimestamp().Return(env.latestForkTime).AnyTimes()
22532353
env.state.EXPECT().GetSubnetOwner(env.unsignedTx.Subnet).Return(subnetOwner, nil).Times(1)
2354+
env.state.EXPECT().GetSubnetManager(env.unsignedTx.Subnet).Return(ids.Empty, nil, database.ErrNotFound).Times(1)
22542355
env.state.EXPECT().GetSubnetTransformation(env.unsignedTx.Subnet).Return(nil, database.ErrNotFound).Times(1)
22552356
env.fx.EXPECT().VerifyPermission(env.unsignedTx, env.unsignedTx.SubnetAuth, env.tx.Creds[len(env.tx.Creds)-1], subnetOwner).Return(nil).Times(1)
22562357
env.flowChecker.EXPECT().VerifySpend(

‎vms/platformvm/txs/executor/subnet_tx_verification.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ func verifyPoASubnetAuthorization(
4343
return nil, err
4444
}
4545

46+
_, _, err = chainState.GetSubnetManager(subnetID)
47+
if err == nil {
48+
return nil, fmt.Errorf("%q %w", subnetID, errIsImmutable)
49+
}
50+
if err != database.ErrNotFound {
51+
return nil, err
52+
}
53+
4654
return creds, nil
4755
}
4856

0 commit comments

Comments
 (0)