Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PVM] Add the functionality of multisig alias definition removal in MultisigAliasTx #377

Merged
merged 1 commit into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions vms/components/multisig/camino_multisig.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package multisig

import (
"errors"
"fmt"

"github.com/ava-labs/avalanchego/ids"
Expand All @@ -16,10 +17,23 @@ import (
// MaxMemoSize is the maximum number of bytes in the memo field
const MaxMemoSize = 256

var (
_ verify.State = (*Alias)(nil)
mo-c4t marked this conversation as resolved.
Show resolved Hide resolved

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
mo-c4t marked this conversation as resolved.
Show resolved Hide resolved

IsZero() bool
}

type Alias struct {
ID ids.ShortID `serialize:"true" json:"id"`
mo-c4t marked this conversation as resolved.
Show resolved Hide resolved
Memo types.JSONByteSlice `serialize:"true" json:"memo"`
Owners verify.State `serialize:"true" json:"owners"`
Owners Owners `serialize:"true" json:"owners"`
}

type AliasWithNonce struct {
Expand All @@ -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()
Expand Down
35 changes: 22 additions & 13 deletions vms/components/multisig/camino_multisig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
4 changes: 2 additions & 2 deletions vms/platformvm/dac/camino_add_member_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/dac/camino_exclude_member_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/dac/camino_general_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]) {
Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/dac/camino_general_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/state/camino.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ type CaminoDiff interface {
// Multisig Owners

GetMultisigAlias(ids.ShortID) (*multisig.AliasWithNonce, error)
SetMultisigAlias(*multisig.AliasWithNonce)
SetMultisigAlias(ids.ShortID, *multisig.AliasWithNonce)

// ShortIDsLink

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/state/camino_address_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions vms/platformvm/state/camino_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/state/camino_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions vms/platformvm/state/camino_multisig_alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions vms/platformvm/state/camino_multisig_alias_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/state/camino_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions vms/platformvm/state/mock_chain.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions vms/platformvm/state/mock_diff.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions vms/platformvm/state/mock_state.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vms/platformvm/txs/camino_deposit_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/txs/camino_deposit_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions vms/platformvm/txs/camino_multisig_alias_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading