From fdcd57746963d7763fa2c7e95f07b6861d2bd07d Mon Sep 17 00:00:00 2001 From: evlekht Date: Thu, 25 Jul 2024 17:31:19 +0400 Subject: [PATCH] [PVM] Msig alias removal --- vms/components/multisig/camino_multisig.go | 23 ++- .../multisig/camino_multisig_test.go | 35 ++-- .../dac/camino_add_member_proposal_test.go | 4 +- .../camino_exclude_member_proposal_test.go | 4 +- vms/platformvm/dac/camino_general_proposal.go | 6 +- .../dac/camino_general_proposal_test.go | 6 +- vms/platformvm/state/camino.go | 4 +- vms/platformvm/state/camino_address_state.go | 2 +- vms/platformvm/state/camino_diff.go | 8 +- vms/platformvm/state/camino_diff_test.go | 6 +- vms/platformvm/state/camino_multisig_alias.go | 9 +- .../state/camino_multisig_alias_test.go | 5 +- vms/platformvm/state/camino_state.go | 4 +- vms/platformvm/state/mock_chain.go | 8 +- vms/platformvm/state/mock_diff.go | 8 +- vms/platformvm/state/mock_state.go | 8 +- vms/platformvm/txs/camino_deposit_tx.go | 2 +- vms/platformvm/txs/camino_deposit_tx_test.go | 2 +- .../txs/camino_multisig_alias_tx.go | 5 +- .../txs/executor/camino_tx_executor.go | 65 ++++--- .../txs/executor/camino_tx_executor_test.go | 171 ++++++++++++++---- vms/secp256k1fx/output_owners.go | 4 + 22 files changed, 265 insertions(+), 124 deletions(-) diff --git a/vms/components/multisig/camino_multisig.go b/vms/components/multisig/camino_multisig.go index 781feecf43b1..d07b6202a000 100644 --- a/vms/components/multisig/camino_multisig.go +++ b/vms/components/multisig/camino_multisig.go @@ -4,6 +4,7 @@ package multisig import ( + "errors" "fmt" "github.com/ava-labs/avalanchego/ids" @@ -16,10 +17,23 @@ import ( // MaxMemoSize is the maximum number of bytes in the memo field const MaxMemoSize = 256 +var ( + _ verify.State = (*Alias)(nil) + + errMemoIsTooBig = errors.New("msig alias memo is too big") + errEmptyAlias = errors.New("alias id and alias owners cannot be empty both at the same time") +) + +type Owners interface { + verify.State + + IsZero() bool +} + type Alias struct { ID ids.ShortID `serialize:"true" json:"id"` Memo types.JSONByteSlice `serialize:"true" json:"memo"` - Owners verify.State `serialize:"true" json:"owners"` + Owners Owners `serialize:"true" json:"owners"` } type AliasWithNonce struct { @@ -34,8 +48,11 @@ func (ma *Alias) InitCtx(ctx *snow.Context) { } func (ma *Alias) Verify() error { - if len(ma.Memo) > MaxMemoSize { - return fmt.Errorf("msig alias memo is larger (%d bytes) than max of %d bytes", len(ma.Memo), MaxMemoSize) + switch { + case len(ma.Memo) > MaxMemoSize: + return fmt.Errorf("%w: expected not greater than %d bytes, got %d bytes", errMemoIsTooBig, MaxMemoSize, len(ma.Memo)) + case ma.Owners.IsZero() && ma.ID == ids.ShortEmpty: + return errEmptyAlias } return ma.Owners.Verify() diff --git a/vms/components/multisig/camino_multisig_test.go b/vms/components/multisig/camino_multisig_test.go index a9072a16be21..3fa625e4c87d 100644 --- a/vms/components/multisig/camino_multisig_test.go +++ b/vms/components/multisig/camino_multisig_test.go @@ -12,31 +12,40 @@ import ( "github.com/stretchr/testify/require" ) +var _ Owners = (*testOwners)(nil) + +type testOwners struct { + avax.TestVerifiable + isEmpty bool +} + +func (o testOwners) IsZero() bool { return o.isEmpty } + func TestVerify(t *testing.T) { tests := map[string]struct { - alias Alias - message string - expectedErrorString string + alias Alias + expectedErr error }{ - "MemoSizeShouldBeLowerThanMaxMemoSize": { + "Memo size should be lower than maxMemoSize": { alias: Alias{ - Owners: &avax.TestVerifiable{}, + Owners: &testOwners{}, Memo: make([]byte, avax.MaxMemoSize+1), ID: hashing.ComputeHash160Array(ids.Empty[:]), }, - message: "memo size should be lower than max memo size", - expectedErrorString: "msig alias memo is larger (257 bytes) than max of 256 bytes", + expectedErr: errMemoIsTooBig, + }, + "Zero owners": { + alias: Alias{ + ID: ids.ShortEmpty, + Owners: &testOwners{isEmpty: true}, + }, + expectedErr: errEmptyAlias, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { err := tt.alias.Verify() - if tt.expectedErrorString != "" { - require.Error(t, err, tt.message) - require.Equal(t, err.Error(), tt.expectedErrorString) - } else { - require.NoError(t, err, tt.message) - } + require.ErrorIs(t, err, tt.expectedErr) }) } } diff --git a/vms/platformvm/dac/camino_add_member_proposal_test.go b/vms/platformvm/dac/camino_add_member_proposal_test.go index 36c2f62b722c..c1399351bc6a 100644 --- a/vms/platformvm/dac/camino_add_member_proposal_test.go +++ b/vms/platformvm/dac/camino_add_member_proposal_test.go @@ -38,7 +38,7 @@ func TestAddMemberProposalVerify(t *testing.T) { }, expectedErr: errEndNotAfterStart, }, - "To small duration": { + "Too small duration": { proposal: &AddMemberProposal{ Start: 100, End: 100 + AddMemberProposalDuration - 1, @@ -49,7 +49,7 @@ func TestAddMemberProposalVerify(t *testing.T) { }, expectedErr: errWrongDuration, }, - "To big duration": { + "Too big duration": { proposal: &AddMemberProposal{ Start: 100, End: 100 + AddMemberProposalDuration + 1, diff --git a/vms/platformvm/dac/camino_exclude_member_proposal_test.go b/vms/platformvm/dac/camino_exclude_member_proposal_test.go index 667fa6adb938..8c504e814feb 100644 --- a/vms/platformvm/dac/camino_exclude_member_proposal_test.go +++ b/vms/platformvm/dac/camino_exclude_member_proposal_test.go @@ -38,7 +38,7 @@ func TestExcludeMemberProposalVerify(t *testing.T) { }, expectedErr: errEndNotAfterStart, }, - "To small duration": { + "Too small duration": { proposal: &ExcludeMemberProposal{ Start: 100, End: 100 + ExcludeMemberProposalMinDuration - 1, @@ -49,7 +49,7 @@ func TestExcludeMemberProposalVerify(t *testing.T) { }, expectedErr: errWrongDuration, }, - "To big duration": { + "Too big duration": { proposal: &ExcludeMemberProposal{ Start: 100, End: 100 + ExcludeMemberProposalMaxDuration + 1, diff --git a/vms/platformvm/dac/camino_general_proposal.go b/vms/platformvm/dac/camino_general_proposal.go index a976e7bf5329..52ec479d28a3 100644 --- a/vms/platformvm/dac/camino_general_proposal.go +++ b/vms/platformvm/dac/camino_general_proposal.go @@ -27,8 +27,8 @@ var ( _ Proposal = (*GeneralProposal)(nil) _ ProposalState = (*GeneralProposalState)(nil) - errGeneralProposalOptionIsToBig = errors.New("option size is to big") - errNotImplemented = errors.New("not implemented, should never be called") + errGeneralProposalOptionIsTooBig = errors.New("option size is too big") + errNotImplemented = errors.New("not implemented, should never be called") ) type GeneralProposal struct { @@ -83,7 +83,7 @@ func (p *GeneralProposal) Verify() error { for i := 0; i < len(p.Options); i++ { if len(p.Options[i]) > generalProposalMaxOptionSize { return fmt.Errorf("%w (expected: no more than %d, actual: %d, option index: %d)", - errGeneralProposalOptionIsToBig, generalProposalMaxOptionSize, len(p.Options[i]), i) + errGeneralProposalOptionIsTooBig, generalProposalMaxOptionSize, len(p.Options[i]), i) } for j := i + 1; j < len(p.Options); j++ { if bytes.Equal(p.Options[i], p.Options[j]) { diff --git a/vms/platformvm/dac/camino_general_proposal_test.go b/vms/platformvm/dac/camino_general_proposal_test.go index 5224900d1a43..e5d24580ef38 100644 --- a/vms/platformvm/dac/camino_general_proposal_test.go +++ b/vms/platformvm/dac/camino_general_proposal_test.go @@ -68,7 +68,7 @@ func TestGeneralProposalVerify(t *testing.T) { }, expectedErr: errEndNotAfterStart, }, - "To small duration": { + "Too small duration": { proposal: &GeneralProposal{ Start: 100, End: 100 + GeneralProposalMinDuration - 1, @@ -81,7 +81,7 @@ func TestGeneralProposalVerify(t *testing.T) { }, expectedErr: errWrongDuration, }, - "To big duration": { + "Too big duration": { proposal: &GeneralProposal{ Start: 100, End: 100 + generalProposalMaxDuration + 1, @@ -105,7 +105,7 @@ func TestGeneralProposalVerify(t *testing.T) { End: 100 + GeneralProposalMinDuration, Options: [][]byte{make([]byte, generalProposalMaxOptionSize+1)}, }, - expectedErr: errGeneralProposalOptionIsToBig, + expectedErr: errGeneralProposalOptionIsTooBig, }, "Not unique option": { proposal: &GeneralProposal{ diff --git a/vms/platformvm/state/camino.go b/vms/platformvm/state/camino.go index f6dc553fe4f6..d05896c02e33 100644 --- a/vms/platformvm/state/camino.go +++ b/vms/platformvm/state/camino.go @@ -108,7 +108,7 @@ type CaminoDiff interface { // Multisig Owners GetMultisigAlias(ids.ShortID) (*multisig.AliasWithNonce, error) - SetMultisigAlias(*multisig.AliasWithNonce) + SetMultisigAlias(ids.ShortID, *multisig.AliasWithNonce) // ShortIDsLink @@ -425,7 +425,7 @@ func (cs *caminoState) syncGenesis(s *state, g *genesis.State) error { // adding msig aliases for _, multisigAlias := range g.Camino.MultisigAliases { - cs.SetMultisigAlias(&multisig.AliasWithNonce{Alias: *multisigAlias}) + cs.SetMultisigAlias(multisigAlias.ID, &multisig.AliasWithNonce{Alias: *multisigAlias}) } // adding blocks (validators and deposits) diff --git a/vms/platformvm/state/camino_address_state.go b/vms/platformvm/state/camino_address_state.go index 834da1523779..c0d64063686f 100644 --- a/vms/platformvm/state/camino_address_state.go +++ b/vms/platformvm/state/camino_address_state.go @@ -44,7 +44,7 @@ func (cs *caminoState) GetAddressStates(address ids.ShortID) (as.AddressState, e func (cs *caminoState) writeAddressStates() error { for key, val := range cs.modifiedAddressStates { delete(cs.modifiedAddressStates, key) - if val == 0 { + if val == as.AddressStateEmpty { if err := cs.addressStateDB.Delete(key[:]); err != nil { return err } diff --git a/vms/platformvm/state/camino_diff.go b/vms/platformvm/state/camino_diff.go index f51881827dad..d4c271ccd04e 100644 --- a/vms/platformvm/state/camino_diff.go +++ b/vms/platformvm/state/camino_diff.go @@ -273,8 +273,8 @@ func (d *diff) GetNextToUnlockDepositIDsAndTime(removedDepositIDs set.Set[ids.ID return nextDepositIDs, nextUnlockTime, nil } -func (d *diff) SetMultisigAlias(alias *multisig.AliasWithNonce) { - d.caminoDiff.modifiedMultisigAliases[alias.ID] = alias +func (d *diff) SetMultisigAlias(id ids.ShortID, alias *multisig.AliasWithNonce) { + d.caminoDiff.modifiedMultisigAliases[id] = alias } func (d *diff) GetMultisigAlias(alias ids.ShortID) (*multisig.AliasWithNonce, error) { @@ -682,8 +682,8 @@ func (d *diff) ApplyCaminoState(baseState State) error { } } - for _, v := range d.caminoDiff.modifiedMultisigAliases { - baseState.SetMultisigAlias(v) + for multisigAliasID, multisigAlias := range d.caminoDiff.modifiedMultisigAliases { + baseState.SetMultisigAlias(multisigAliasID, multisigAlias) } for fullKey, link := range d.caminoDiff.modifiedShortLinks { diff --git a/vms/platformvm/state/camino_diff_test.go b/vms/platformvm/state/camino_diff_test.go index 274af81cb765..25a059b811fc 100644 --- a/vms/platformvm/state/camino_diff_test.go +++ b/vms/platformvm/state/camino_diff_test.go @@ -2383,7 +2383,7 @@ func TestDiffSetMultisigAlias(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - tt.diff.SetMultisigAlias(tt.alias) + tt.diff.SetMultisigAlias(tt.alias.ID, tt.alias) require.Equal(t, tt.expectedDiff, tt.diff) }) } @@ -5185,8 +5185,8 @@ func TestDiffApplyCaminoState(t *testing.T) { s.EXPECT().ModifyDeposit(depositTxID, depositDiff.Deposit) } } - for _, v := range d.caminoDiff.modifiedMultisigAliases { - s.EXPECT().SetMultisigAlias(v) + for multisigAliasID, multisigAlias := range d.caminoDiff.modifiedMultisigAliases { + s.EXPECT().SetMultisigAlias(multisigAliasID, multisigAlias) } for fullKey, link := range d.caminoDiff.modifiedShortLinks { id, key := fromShortLinkKey(fullKey) diff --git a/vms/platformvm/state/camino_multisig_alias.go b/vms/platformvm/state/camino_multisig_alias.go index e2f72e469e01..e86e1b50a987 100644 --- a/vms/platformvm/state/camino_multisig_alias.go +++ b/vms/platformvm/state/camino_multisig_alias.go @@ -9,20 +9,19 @@ import ( "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/vms/components/multisig" - "github.com/ava-labs/avalanchego/vms/components/verify" "github.com/ava-labs/avalanchego/vms/platformvm/blocks" "github.com/ava-labs/avalanchego/vms/types" ) type msigAlias struct { Memo types.JSONByteSlice `serialize:"true"` - Owners verify.State `serialize:"true"` + Owners multisig.Owners `serialize:"true"` Nonce uint64 `serialize:"true"` } -func (cs *caminoState) SetMultisigAlias(ma *multisig.AliasWithNonce) { - cs.modifiedMultisigAliases[ma.ID] = ma - cs.multisigAliasesCache.Evict(ma.ID) +func (cs *caminoState) SetMultisigAlias(id ids.ShortID, ma *multisig.AliasWithNonce) { + cs.modifiedMultisigAliases[id] = ma + cs.multisigAliasesCache.Evict(id) } func (cs *caminoState) GetMultisigAlias(id ids.ShortID) (*multisig.AliasWithNonce, error) { diff --git a/vms/platformvm/state/camino_multisig_alias_test.go b/vms/platformvm/state/camino_multisig_alias_test.go index a91ca6602ef6..90eceb32a255 100644 --- a/vms/platformvm/state/camino_multisig_alias_test.go +++ b/vms/platformvm/state/camino_multisig_alias_test.go @@ -222,9 +222,8 @@ func TestSetMultisigAlias(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - caminoState := tt.caminoState(ctrl) - caminoState.SetMultisigAlias(tt.multisigAlias) + caminoState := tt.caminoState(gomock.NewController(t)) + caminoState.SetMultisigAlias(tt.multisigAlias.ID, tt.multisigAlias) require.Equal(t, tt.expectedCaminoState(caminoState), caminoState) }) } diff --git a/vms/platformvm/state/camino_state.go b/vms/platformvm/state/camino_state.go index 9e33d6ab19c8..22b8ef894127 100644 --- a/vms/platformvm/state/camino_state.go +++ b/vms/platformvm/state/camino_state.go @@ -94,8 +94,8 @@ func (s *state) GetNextToUnlockDepositIDsAndTime(removedDepositIDs set.Set[ids.I return s.caminoState.GetNextToUnlockDepositIDsAndTime(removedDepositIDs) } -func (s *state) SetMultisigAlias(owner *multisig.AliasWithNonce) { - s.caminoState.SetMultisigAlias(owner) +func (s *state) SetMultisigAlias(id ids.ShortID, owner *multisig.AliasWithNonce) { + s.caminoState.SetMultisigAlias(id, owner) } func (s *state) GetMultisigAlias(alias ids.ShortID) (*multisig.AliasWithNonce, error) { diff --git a/vms/platformvm/state/mock_chain.go b/vms/platformvm/state/mock_chain.go index 79aff08722d8..6d5064ef00df 100644 --- a/vms/platformvm/state/mock_chain.go +++ b/vms/platformvm/state/mock_chain.go @@ -969,15 +969,15 @@ func (mr *MockChainMockRecorder) SetLastRewardImportTimestamp(arg0 interface{}) } // SetMultisigAlias mocks base method. -func (m *MockChain) SetMultisigAlias(arg0 *multisig.AliasWithNonce) { +func (m *MockChain) SetMultisigAlias(arg0 ids.ShortID, arg1 *multisig.AliasWithNonce) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetMultisigAlias", arg0) + m.ctrl.Call(m, "SetMultisigAlias", arg0, arg1) } // SetMultisigAlias indicates an expected call of SetMultisigAlias. -func (mr *MockChainMockRecorder) SetMultisigAlias(arg0 interface{}) *gomock.Call { +func (mr *MockChainMockRecorder) SetMultisigAlias(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockChain)(nil).SetMultisigAlias), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockChain)(nil).SetMultisigAlias), arg0, arg1) } // SetNotDistributedValidatorReward mocks base method. diff --git a/vms/platformvm/state/mock_diff.go b/vms/platformvm/state/mock_diff.go index 19c4b39acb0a..edccc5a9a69d 100644 --- a/vms/platformvm/state/mock_diff.go +++ b/vms/platformvm/state/mock_diff.go @@ -984,15 +984,15 @@ func (mr *MockDiffMockRecorder) SetDelegateeReward(arg0, arg1, arg2 interface{}) } // SetMultisigAlias mocks base method. -func (m *MockDiff) SetMultisigAlias(arg0 *multisig.AliasWithNonce) { +func (m *MockDiff) SetMultisigAlias(arg0 ids.ShortID, arg1 *multisig.AliasWithNonce) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetMultisigAlias", arg0) + m.ctrl.Call(m, "SetMultisigAlias", arg0, arg1) } // SetMultisigAlias indicates an expected call of SetMultisigAlias. -func (mr *MockDiffMockRecorder) SetMultisigAlias(arg0 interface{}) *gomock.Call { +func (mr *MockDiffMockRecorder) SetMultisigAlias(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockDiff)(nil).SetMultisigAlias), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockDiff)(nil).SetMultisigAlias), arg0, arg1) } // SetNotDistributedValidatorReward mocks base method. diff --git a/vms/platformvm/state/mock_state.go b/vms/platformvm/state/mock_state.go index 1290289e14ad..ad336ff7a2c9 100644 --- a/vms/platformvm/state/mock_state.go +++ b/vms/platformvm/state/mock_state.go @@ -1145,15 +1145,15 @@ func (mr *MockStateMockRecorder) SetLastAccepted(arg0 interface{}) *gomock.Call } // SetMultisigAlias mocks base method. -func (m *MockState) SetMultisigAlias(arg0 *multisig.AliasWithNonce) { +func (m *MockState) SetMultisigAlias(arg0 ids.ShortID, arg1 *multisig.AliasWithNonce) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetMultisigAlias", arg0) + m.ctrl.Call(m, "SetMultisigAlias", arg0, arg1) } // SetMultisigAlias indicates an expected call of SetMultisigAlias. -func (mr *MockStateMockRecorder) SetMultisigAlias(arg0 interface{}) *gomock.Call { +func (mr *MockStateMockRecorder) SetMultisigAlias(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockState)(nil).SetMultisigAlias), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMultisigAlias", reflect.TypeOf((*MockState)(nil).SetMultisigAlias), arg0, arg1) } // SetNotDistributedValidatorReward mocks base method. diff --git a/vms/platformvm/txs/camino_deposit_tx.go b/vms/platformvm/txs/camino_deposit_tx.go index 08b27b18ff8b..f473d23fa22c 100644 --- a/vms/platformvm/txs/camino_deposit_tx.go +++ b/vms/platformvm/txs/camino_deposit_tx.go @@ -19,7 +19,7 @@ import ( var ( _ UnsignedTx = (*DepositTx)(nil) - errTooBigDeposit = errors.New("to big deposit") + errTooBigDeposit = errors.New("too big deposit") errInvalidRewardOwner = errors.New("invalid reward owner") errBadOfferOwnerAuth = errors.New("bad offer owner auth") errBadDepositCreatorAuth = errors.New("bad deposit creator auth") diff --git a/vms/platformvm/txs/camino_deposit_tx_test.go b/vms/platformvm/txs/camino_deposit_tx_test.go index a21b1f867a96..f0468c55c4e4 100644 --- a/vms/platformvm/txs/camino_deposit_tx_test.go +++ b/vms/platformvm/txs/camino_deposit_tx_test.go @@ -37,7 +37,7 @@ func TestDepositTxSyntacticVerify(t *testing.T) { }, expectedErr: errInvalidRewardOwner, }, - "To big total deposit amount": { + "Too big total deposit amount": { tx: &DepositTx{ BaseTx: BaseTx{BaseTx: avax.BaseTx{ NetworkID: ctx.NetworkID, diff --git a/vms/platformvm/txs/camino_multisig_alias_tx.go b/vms/platformvm/txs/camino_multisig_alias_tx.go index 374669706703..b94d68d8db7e 100644 --- a/vms/platformvm/txs/camino_multisig_alias_tx.go +++ b/vms/platformvm/txs/camino_multisig_alias_tx.go @@ -14,8 +14,9 @@ import ( ) var ( - _ UnsignedTx = (*MultisigAliasTx)(nil) - errFailedToVerifyAliasOrAuth = errors.New("failed to verify alias or auth") + _ UnsignedTx = (*MultisigAliasTx)(nil) + + errFailedToVerifyAliasOrAuth = errors.New("failed to verify alias or auth") ) // MultisigAliasTx is an unsigned multisig alias tx diff --git a/vms/platformvm/txs/executor/camino_tx_executor.go b/vms/platformvm/txs/executor/camino_tx_executor.go index 59077aa73efb..361a122b07fb 100644 --- a/vms/platformvm/txs/executor/camino_tx_executor.go +++ b/vms/platformvm/txs/executor/camino_tx_executor.go @@ -54,6 +54,7 @@ var ( errDepositDurationTooBig = errors.New("deposit duration is greater than deposit offer maximum duration") errSupplyOverflow = errors.New("resulting total supply would be more than allowed maximum") errNotConsortiumMember = errors.New("address isn't consortium member") + errConsortiumMember = errors.New("address is consortium member") errValidatorNotFound = errors.New("validator not found") errConsortiumMemberHasNode = errors.New("consortium member already has registered node") errSignatureMissing = errors.New("wrong signature") @@ -1514,15 +1515,7 @@ func (e *CaminoStandardTxExecutor) MultisigAliasTx(tx *txs.MultisigAliasTx) erro return err } - baseCreds := e.Tx.Creds[:len(e.Tx.Creds)] - - var aliasID ids.ShortID - nonce := uint64(0) - - txID := e.Tx.ID() - // verify that alias isn't nesting another alias - isNestedMsig, err := e.Fx.IsNestedMultisig(tx.MultisigAlias.Owners, e.State) switch { case err != nil: @@ -1531,14 +1524,21 @@ func (e *CaminoStandardTxExecutor) MultisigAliasTx(tx *txs.MultisigAliasTx) erro return errNestedMsigAlias } - // Update existing multisig definition + aliasAddrState := as.AddressStateEmpty + baseCreds := e.Tx.Creds[:len(e.Tx.Creds)] + var aliasID ids.ShortID + nonce := uint64(0) + txID := e.Tx.ID() + isRemoval := tx.MultisigAlias.Owners.IsZero() + isUpdating := tx.MultisigAlias.ID != ids.ShortEmpty + // syntactically valid multisig alias ensures that is removal is also updating - if tx.MultisigAlias.ID != ids.ShortEmpty { + if isUpdating { if len(e.Tx.Creds) < 2 { return errWrongCredentialsNumber } - alias, err := e.State.GetMultisigAlias(tx.MultisigAlias.ID) + oldAlias, err := e.State.GetMultisigAlias(tx.MultisigAlias.ID) if err != nil { return fmt.Errorf("%w, alias: %s", errAliasNotFound, tx.MultisigAlias.ID) } @@ -1549,14 +1549,28 @@ func (e *CaminoStandardTxExecutor) MultisigAliasTx(tx *txs.MultisigAliasTx) erro e.Tx.Unsigned, tx.Auth, e.Tx.Creds[len(e.Tx.Creds)-1], - alias.Owners, + oldAlias.Owners, e.State, ); err != nil { return fmt.Errorf("%w: %s", errAliasCredentialMismatch, err) } - aliasID = alias.ID - nonce = alias.Nonce + 1 + if isRemoval { + // verify that alias isn't consortium member or role admin + aliasAddrState, err = e.State.GetAddressStates(tx.MultisigAlias.ID) + switch { + case err != nil: + return err + case aliasAddrState.Is(as.AddressStateConsortium): + return errConsortiumMember + case aliasAddrState.Is(as.AddressStateRoleAdmin): + return errAdminCannotBeDeleted + } + } else { + nonce = oldAlias.Nonce + 1 + } + + aliasID = oldAlias.ID } else { aliasID = multisig.ComputeAliasID(txID) } @@ -1583,14 +1597,21 @@ func (e *CaminoStandardTxExecutor) MultisigAliasTx(tx *txs.MultisigAliasTx) erro // update state - e.State.SetMultisigAlias(&multisig.AliasWithNonce{ - Alias: multisig.Alias{ - ID: aliasID, - Memo: tx.MultisigAlias.Memo, - Owners: tx.MultisigAlias.Owners, - }, - Nonce: nonce, - }) + var msigAlias *multisig.AliasWithNonce + if isRemoval && aliasAddrState != as.AddressStateEmpty { + e.State.SetAddressStates(tx.MultisigAlias.ID, as.AddressStateEmpty) + } else if !isRemoval { + msigAlias = &multisig.AliasWithNonce{ + Alias: multisig.Alias{ + ID: aliasID, + Memo: tx.MultisigAlias.Memo, + Owners: tx.MultisigAlias.Owners, + }, + Nonce: nonce, + } + } + + e.State.SetMultisigAlias(aliasID, msigAlias) // Consume the UTXOS avax.Consume(e.State, tx.Ins) diff --git a/vms/platformvm/txs/executor/camino_tx_executor_test.go b/vms/platformvm/txs/executor/camino_tx_executor_test.go index 49208fc9d426..f2c3993d9589 100644 --- a/vms/platformvm/txs/executor/camino_tx_executor_test.go +++ b/vms/platformvm/txs/executor/camino_tx_executor_test.go @@ -5383,6 +5383,14 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { ownerUTXO := generate.UTXO(ids.GenerateTestID(), ctx.AVAXAssetID, test.TxFee, owner, ids.Empty, ids.Empty, true) msigUTXO := generate.UTXO(ids.GenerateTestID(), ctx.AVAXAssetID, test.TxFee, *msigOwner, ids.Empty, ids.Empty, true) + baseTx := txs.BaseTx{ + BaseTx: avax.BaseTx{ + NetworkID: ctx.NetworkID, + BlockchainID: ctx.ChainID, + Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, + }, + } + caminoGenesisConf := api.Camino{ VerifyNodeSignature: true, LockModeBondDeposit: true, @@ -5401,13 +5409,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { return s }, utx: &txs.MultisigAliasTx{ - BaseTx: txs.BaseTx{ - BaseTx: avax.BaseTx{ - NetworkID: ctx.NetworkID, - BlockchainID: ctx.ChainID, - Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, - }, - }, + BaseTx: baseTx, MultisigAlias: multisig.Alias{ ID: ids.ShortID{}, Memo: msigAlias.Memo, @@ -5428,13 +5430,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { return s }, utx: &txs.MultisigAliasTx{ - BaseTx: txs.BaseTx{ - BaseTx: avax.BaseTx{ - NetworkID: ctx.NetworkID, - BlockchainID: ctx.ChainID, - Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, - }, - }, + BaseTx: baseTx, MultisigAlias: msigAlias.Alias, Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, }, @@ -5456,13 +5452,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { return s }, utx: &txs.MultisigAliasTx{ - BaseTx: txs.BaseTx{ - BaseTx: avax.BaseTx{ - NetworkID: ctx.NetworkID, - BlockchainID: ctx.ChainID, - Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, - }, - }, + BaseTx: baseTx, MultisigAlias: msigAlias.Alias, Auth: &secp256k1fx.Input{SigIndices: []uint32{0}}, }, @@ -5472,6 +5462,56 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { }, expectedErr: errAliasCredentialMismatch, }, + "Removing alias with consortium addr state": { + state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { + s := state.NewMockDiff(c) + s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) + expect.VerifyMultisigPermission(t, s, []ids.ShortID{ + msigAliasOwners.Addrs[0], + msigAliasOwners.Addrs[1], + }, []*multisig.AliasWithNonce{}) + s.EXPECT().GetAddressStates(msigAlias.Alias.ID).Return(as.AddressStateConsortium, nil) + return s + }, + utx: &txs.MultisigAliasTx{ + BaseTx: baseTx, + MultisigAlias: multisig.Alias{ + ID: msigAlias.ID, + Owners: &secp256k1fx.OutputOwners{}, + }, + Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, + }, + signers: [][]*secp256k1.PrivateKey{ + {ownerKey}, + {msigKeys[0], msigKeys[1]}, + }, + expectedErr: errConsortiumMember, + }, + "Removing alias with role admin addr state": { + state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { + s := state.NewMockDiff(c) + s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) + expect.VerifyMultisigPermission(t, s, []ids.ShortID{ + msigAliasOwners.Addrs[0], + msigAliasOwners.Addrs[1], + }, []*multisig.AliasWithNonce{}) + s.EXPECT().GetAddressStates(msigAlias.Alias.ID).Return(as.AddressStateRoleAdmin, nil) + return s + }, + utx: &txs.MultisigAliasTx{ + BaseTx: baseTx, + MultisigAlias: multisig.Alias{ + ID: msigAlias.ID, + Owners: &secp256k1fx.OutputOwners{}, + }, + Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, + }, + signers: [][]*secp256k1.PrivateKey{ + {ownerKey}, + {msigKeys[0], msigKeys[1]}, + }, + expectedErr: errAdminCannotBeDeleted, + }, "OK, update existing alias": { state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) @@ -5483,7 +5523,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { }, []*multisig.AliasWithNonce{}) s.EXPECT().GetBaseFee().Return(test.TxFee, nil) expect.VerifyLock(t, s, utx.Ins, []*avax.UTXO{ownerUTXO}, []ids.ShortID{ownerAddr}, nil) - s.EXPECT().SetMultisigAlias(&multisig.AliasWithNonce{ + s.EXPECT().SetMultisigAlias(msigAlias.ID, &multisig.AliasWithNonce{ Alias: msigAlias.Alias, Nonce: msigAlias.Nonce + 1, }) @@ -5492,13 +5532,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { return s }, utx: &txs.MultisigAliasTx{ - BaseTx: txs.BaseTx{ - BaseTx: avax.BaseTx{ - NetworkID: ctx.NetworkID, - BlockchainID: ctx.ChainID, - Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, - }, - }, + BaseTx: baseTx, MultisigAlias: msigAlias.Alias, Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, }, @@ -5507,15 +5541,77 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { {msigKeys[0], msigKeys[1]}, }, }, + "OK, remove existing alias with non-empty addr state": { + state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { + s := state.NewMockDiff(c) + s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) + expect.VerifyMultisigPermission(t, s, []ids.ShortID{ + msigAliasOwners.Addrs[0], + msigAliasOwners.Addrs[1], + }, []*multisig.AliasWithNonce{}) + s.EXPECT().GetAddressStates(msigAlias.Alias.ID).Return(as.AddressStateKYCVerified, nil) + s.EXPECT().GetBaseFee().Return(test.TxFee, nil) + expect.VerifyLock(t, s, utx.Ins, []*avax.UTXO{ownerUTXO}, []ids.ShortID{ownerAddr}, nil) + s.EXPECT().SetAddressStates(msigAlias.Alias.ID, as.AddressStateEmpty) + s.EXPECT().SetMultisigAlias(msigAlias.Alias.ID, nil) + expect.ConsumeUTXOs(t, s, utx.Ins) + expect.ProduceUTXOs(t, s, utx.Outs, txID, 0) + return s + }, + utx: &txs.MultisigAliasTx{ + BaseTx: baseTx, + MultisigAlias: multisig.Alias{ + ID: msigAlias.ID, + Memo: msigAlias.Memo, + Owners: &secp256k1fx.OutputOwners{}, + }, + Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, + }, + signers: [][]*secp256k1.PrivateKey{ + {ownerKey}, + {msigKeys[0], msigKeys[1]}, + }, + }, + "OK, remove existing alias with empty addr state": { + state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { + s := state.NewMockDiff(c) + s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) + expect.VerifyMultisigPermission(t, s, []ids.ShortID{ + msigAliasOwners.Addrs[0], + msigAliasOwners.Addrs[1], + }, []*multisig.AliasWithNonce{}) + s.EXPECT().GetAddressStates(msigAlias.Alias.ID).Return(as.AddressStateEmpty, nil) + s.EXPECT().GetBaseFee().Return(test.TxFee, nil) + expect.VerifyLock(t, s, utx.Ins, []*avax.UTXO{ownerUTXO}, []ids.ShortID{ownerAddr}, nil) + s.EXPECT().SetMultisigAlias(msigAlias.Alias.ID, nil) + expect.ConsumeUTXOs(t, s, utx.Ins) + expect.ProduceUTXOs(t, s, utx.Outs, txID, 0) + return s + }, + utx: &txs.MultisigAliasTx{ + BaseTx: baseTx, + MultisigAlias: multisig.Alias{ + ID: msigAlias.ID, + Memo: msigAlias.Memo, + Owners: &secp256k1fx.OutputOwners{}, + }, + Auth: &secp256k1fx.Input{SigIndices: []uint32{0, 1}}, + }, + signers: [][]*secp256k1.PrivateKey{ + {ownerKey}, + {msigKeys[0], msigKeys[1]}, + }, + }, "OK, add new alias": { state: func(t *testing.T, c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) expect.GetMultisigAliases(t, s, msigAliasOwners.Addrs, nil) s.EXPECT().GetBaseFee().Return(test.TxFee, nil) expect.VerifyLock(t, s, utx.Ins, []*avax.UTXO{ownerUTXO}, []ids.ShortID{ownerAddr}, nil) - s.EXPECT().SetMultisigAlias(&multisig.AliasWithNonce{ + aliasID := multisig.ComputeAliasID(txID) + s.EXPECT().SetMultisigAlias(aliasID, &multisig.AliasWithNonce{ Alias: multisig.Alias{ - ID: multisig.ComputeAliasID(txID), + ID: aliasID, Memo: msigAlias.Memo, Owners: msigAlias.Owners, }, @@ -5526,13 +5622,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { return s }, utx: &txs.MultisigAliasTx{ - BaseTx: txs.BaseTx{ - BaseTx: avax.BaseTx{ - NetworkID: ctx.NetworkID, - BlockchainID: ctx.ChainID, - Ins: []*avax.TransferableInput{generate.InFromUTXO(t, ownerUTXO, []uint32{0}, false)}, - }, - }, + BaseTx: baseTx, MultisigAlias: multisig.Alias{ ID: ids.ShortID{}, Memo: msigAlias.Memo, @@ -5554,9 +5644,10 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { msigAliasOwners.Addrs[0], msigAliasOwners.Addrs[1], }, []*multisig.AliasWithNonce{msigAlias}) - s.EXPECT().SetMultisigAlias(&multisig.AliasWithNonce{ + aliasID := multisig.ComputeAliasID(txID) + s.EXPECT().SetMultisigAlias(aliasID, &multisig.AliasWithNonce{ Alias: multisig.Alias{ - ID: multisig.ComputeAliasID(txID), + ID: aliasID, Memo: msigAlias.Memo, Owners: msigAlias.Owners, }, diff --git a/vms/secp256k1fx/output_owners.go b/vms/secp256k1fx/output_owners.go index eb601eb3773e..c0521e7b2f12 100644 --- a/vms/secp256k1fx/output_owners.go +++ b/vms/secp256k1fx/output_owners.go @@ -120,6 +120,10 @@ func (out *OutputOwners) Equals(other *OutputOwners) bool { return true } +func (out *OutputOwners) IsZero() bool { + return out.Equals(&OutputOwners{}) +} + func (out *OutputOwners) Verify() error { switch { case out == nil: