diff --git a/vms/platformvm/fx/camino_fx.go b/vms/platformvm/fx/camino_fx.go index 93c77aea3465..fa9ac592aadd 100644 --- a/vms/platformvm/fx/camino_fx.go +++ b/vms/platformvm/fx/camino_fx.go @@ -31,4 +31,7 @@ type CaminoFx interface { // CollectMultisigAliases returns an array of OutputOwners part of the owner CollectMultisigAliases(ownerIntf, msigIntf interface{}) ([]interface{}, error) + + // Checks if [ownerIntf] contains msig alias + IsNestedMultisig(ownerIntf interface{}, msigIntf interface{}) (bool, error) } diff --git a/vms/platformvm/fx/mock_fx.go b/vms/platformvm/fx/mock_fx.go index d107eb486a17..ceae71ca734d 100644 --- a/vms/platformvm/fx/mock_fx.go +++ b/vms/platformvm/fx/mock_fx.go @@ -210,6 +210,21 @@ func (mr *MockFxMockRecorder) VerifyTransfer(arg0, arg1, arg2, arg3 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VerifyTransfer", reflect.TypeOf((*MockFx)(nil).VerifyTransfer), arg0, arg1, arg2, arg3) } +// IsNestedMultisig mocks base method. +func (m *MockFx) IsNestedMultisig(arg0, arg1 interface{}) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsNestedMultisig", arg0, arg1) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsNestedMultisig indicates an expected call of IsNestedMultisig. +func (mr *MockFxMockRecorder) IsNestedMultisig(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNestedMultisig", reflect.TypeOf((*MockFx)(nil).IsNestedMultisig), arg0, arg1) +} + // MockOwner is a mock of Owner interface. type MockOwner struct { ctrl *gomock.Controller @@ -257,4 +272,4 @@ func (m *MockOwner) Verify() error { func (mr *MockOwnerMockRecorder) Verify() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Verify", reflect.TypeOf((*MockOwner)(nil).Verify)) -} +} \ No newline at end of file diff --git a/vms/platformvm/txs/builder/camino_helpers_test.go b/vms/platformvm/txs/builder/camino_helpers_test.go index ce15bbabf005..1b793ee3d32e 100644 --- a/vms/platformvm/txs/builder/camino_helpers_test.go +++ b/vms/platformvm/txs/builder/camino_helpers_test.go @@ -617,7 +617,7 @@ func expectLock(s *state.MockState, allUTXOs map[ids.ShortID][]*avax.UTXO) { s.EXPECT().UTXOIDs(addr.Bytes(), ids.Empty, math.MaxInt).Return(utxoids, nil) for _, utxo := range utxos { s.EXPECT().GetUTXO(utxo.InputID()).Return(utxo, nil) - s.EXPECT().GetMultisigAlias(addr).Return(nil, database.ErrNotFound) + s.EXPECT().GetMultisigAlias(addr).Return(nil, database.ErrNotFound).Times(2) } } } diff --git a/vms/platformvm/txs/executor/camino_tx_executor.go b/vms/platformvm/txs/executor/camino_tx_executor.go index 8054d55a6bb3..20b17b0b9ef3 100644 --- a/vms/platformvm/txs/executor/camino_tx_executor.go +++ b/vms/platformvm/txs/executor/camino_tx_executor.go @@ -78,6 +78,7 @@ var ( errOfferPermissionCredentialMismatch = errors.New("offer-usage permission credential isn't matching") errEmptyDepositCreatorAddress = errors.New("empty deposit creator address, while offer owner isn't empty") errWrongTxUpgradeVersion = errors.New("wrong tx upgrade version") + errNestedMsigAlias = errors.New("nested msig aliases are not allowed") ) type CaminoStandardTxExecutor struct { @@ -1474,6 +1475,16 @@ func (e *CaminoStandardTxExecutor) MultisigAliasTx(tx *txs.MultisigAliasTx) erro 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: + return err + case isNestedMsig: + return errNestedMsigAlias + } + // Update existing multisig definition if tx.MultisigAlias.ID != ids.ShortEmpty { diff --git a/vms/platformvm/txs/executor/camino_tx_executor_test.go b/vms/platformvm/txs/executor/camino_tx_executor_test.go index 8968726fb585..dbd2510dca85 100644 --- a/vms/platformvm/txs/executor/camino_tx_executor_test.go +++ b/vms/platformvm/txs/executor/camino_tx_executor_test.go @@ -5302,7 +5302,6 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { ownerKey, ownerAddr, owner := generateKeyAndOwner(t) msigKeys, msigAlias, msigAliasOwners, msigOwner := generateMsigAliasAndKeys(t, 2, 2, true) - _, newMsigAlias, _, _ := generateMsigAliasAndKeys(t, 2, 2, true) ownerUTXO := generateTestUTXO(ids.GenerateTestID(), ctx.AVAXAssetID, defaultTxFee, owner, ids.Empty, ids.Empty) msigUTXO := generateTestUTXO(ids.GenerateTestID(), ctx.AVAXAssetID, defaultTxFee, *msigOwner, ids.Empty, ids.Empty) @@ -5318,9 +5317,36 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { signers [][]*secp256k1.PrivateKey expectedErr error }{ + "Add nested alias": { + state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { + s := state.NewMockDiff(c) + s.EXPECT().GetMultisigAlias(msigAliasOwners.Addrs[0]).Return(&multisig.AliasWithNonce{}, nil) + return s + }, + utx: &txs.MultisigAliasTx{ + BaseTx: txs.BaseTx{ + BaseTx: avax.BaseTx{ + NetworkID: ctx.NetworkID, + BlockchainID: ctx.ChainID, + Ins: []*avax.TransferableInput{generateTestInFromUTXO(ownerUTXO, []uint32{0})}, + }, + }, + MultisigAlias: multisig.Alias{ + ID: ids.ShortID{}, + Memo: msigAlias.Memo, + Owners: msigAlias.Owners, + }, + Auth: &secp256k1fx.Input{SigIndices: []uint32{}}, + }, + signers: [][]*secp256k1.PrivateKey{ + {ownerKey}, + }, + expectedErr: errNestedMsigAlias, + }, "Updating alias which does not exist": { state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) + expectGetMultisigAliases(s, msigAliasOwners.Addrs, nil) s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(nil, database.ErrNotFound) return s }, @@ -5344,6 +5370,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { "Updating existing alias with less signatures than threshold": { state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) + expectGetMultisigAliases(s, msigAliasOwners.Addrs, nil) s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) expectVerifyMultisigPermission(s, []ids.ShortID{ msigAliasOwners.Addrs[0], @@ -5371,6 +5398,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { "OK, update existing alias": { state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) + expectGetMultisigAliases(s, msigAliasOwners.Addrs, nil) s.EXPECT().GetMultisigAlias(msigAlias.ID).Return(msigAlias, nil) expectVerifyMultisigPermission(s, []ids.ShortID{ msigAliasOwners.Addrs[0], @@ -5404,6 +5432,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { "OK, add new alias": { state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) + expectGetMultisigAliases(s, msigAliasOwners.Addrs, nil) expectVerifyLock(s, utx.Ins, []*avax.UTXO{ownerUTXO}, []ids.ShortID{ownerAddr}, nil) s.EXPECT().SetMultisigAlias(&multisig.AliasWithNonce{ Alias: multisig.Alias{ @@ -5439,6 +5468,7 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { "OK, add new alias with multisig sender": { state: func(c *gomock.Controller, utx *txs.MultisigAliasTx, txID ids.ID) *state.MockDiff { s := state.NewMockDiff(c) + expectGetMultisigAliases(s, msigAliasOwners.Addrs, nil) expectVerifyLock(s, utx.Ins, []*avax.UTXO{msigUTXO}, []ids.ShortID{ msigAlias.ID, msigAliasOwners.Addrs[0], @@ -5447,8 +5477,8 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { s.EXPECT().SetMultisigAlias(&multisig.AliasWithNonce{ Alias: multisig.Alias{ ID: multisig.ComputeAliasID(txID), - Memo: newMsigAlias.Memo, - Owners: newMsigAlias.Owners, + Memo: msigAlias.Memo, + Owners: msigAlias.Owners, }, Nonce: 0, }) @@ -5466,8 +5496,8 @@ func TestCaminoStandardTxExecutorMultisigAliasTx(t *testing.T) { }, MultisigAlias: multisig.Alias{ ID: ids.ShortID{}, - Memo: newMsigAlias.Memo, - Owners: newMsigAlias.Owners, + Memo: msigAlias.Memo, + Owners: msigAlias.Owners, }, Auth: &secp256k1fx.Input{SigIndices: []uint32{}}, }, diff --git a/vms/platformvm/utxo/camino_locked.go b/vms/platformvm/utxo/camino_locked.go index c0d2a17ab3a0..60e8cb003184 100644 --- a/vms/platformvm/utxo/camino_locked.go +++ b/vms/platformvm/utxo/camino_locked.go @@ -361,7 +361,13 @@ func (h *handler) Lock( if toOwnerID != nil { lockedOwnerID = OwnerID{to, toOwnerID} } - if changeOwnerID != nil && !h.isMultisigTransferOutput(utxoDB, innerOut) { + + isNestedMsig, err := h.fx.IsNestedMultisig(&innerOut.OutputOwners, utxoDB) + if err != nil { + return nil, nil, nil, nil, err + } + + if changeOwnerID != nil && !isNestedMsig { remainingOwnerID = OwnerID{change, changeOwnerID} } } @@ -1247,29 +1253,6 @@ func (h *handler) VerifyUnlockDepositedUTXOs( return nil } -func (*handler) isMultisigTransferOutput(utxoDB avax.UTXOReader, out verify.State) bool { - secpOut, ok := out.(*secp256k1fx.TransferOutput) - if !ok { - // Conversion should succeed, otherwise it will be handled by the caller - return false - } - - // ! because currently there is no support for adding new msig aliases after genesis, - // ! we assume that state diffs won't contain any changes to msig aliases state - // ! that must be changed later - state, ok := utxoDB.(state.CaminoDiff) - if !ok { - return false - } - - for _, addr := range secpOut.Addrs { - if _, err := state.GetMultisigAlias(addr); err == nil { - return true - } - } - return false -} - type innerSortUTXOs struct { utxos []*avax.UTXO allowedAssetID ids.ID diff --git a/vms/secp256k1fx/camino_fx.go b/vms/secp256k1fx/camino_fx.go index 15ec3bcbd7c0..f1a8c459654d 100644 --- a/vms/secp256k1fx/camino_fx.go +++ b/vms/secp256k1fx/camino_fx.go @@ -75,6 +75,29 @@ func (fx *CaminoFx) VerifyTransfer(txIntf, inIntf, credIntf, utxoIntf interface{ return fx.Fx.VerifyTransfer(txIntf, inIntf, credIntf, utxoIntf) } +func (*Fx) IsNestedMultisig(ownerIntf interface{}, msigIntf interface{}) (bool, error) { + owner, ok := ownerIntf.(*OutputOwners) + if !ok { + return false, ErrWrongOwnerType + } + msig, ok := msigIntf.(AliasGetter) + if !ok { + return false, ErrNotAliasGetter + } + + for _, addr := range owner.Addrs { + _, err := msig.GetMultisigAlias(addr) + switch { + case err == nil: + return true, nil + case err != database.ErrNotFound: + return false, err + } + } + + return false, nil +} + func (fx *Fx) RecoverAddresses(msg []byte, verifies []verify.Verifiable) (RecoverMap, error) { ret := make(RecoverMap, len(verifies)) visited := make(map[[secp256k1.SignatureLen]byte]bool)