From ed4698db49ced8a09d0136dc9fa0e68e8e8c32d4 Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Mon, 16 Oct 2023 12:18:52 +0100 Subject: [PATCH 1/3] fix: validation on governance proposals Signed-off-by: Elias Van Ootegem --- CHANGELOG.md | 1 + commands/proposal_submission.go | 99 ++++++++++++------- .../proposal_submission_new_transfer_test.go | 69 ++++++------- 3 files changed, 99 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e74bccce..cba3d62909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -317,6 +317,7 @@ - [9751](https://github.com/vegaprotocol/vega/issues/9751) - Make sure that LP fee party accounts exists. - [9762](https://github.com/vegaprotocol/vega/issues/9762) - Referral fees API not filtering by party correctly. - [9775](https://github.com/vegaprotocol/vega/issues/9775) - Do not pay discount if set is not eligible +- [9788](https://github.com/vegaprotocol/vega/issues/9788) - Fix transfer account validation. ## 0.72.1 diff --git a/commands/proposal_submission.go b/commands/proposal_submission.go index 09289d1d82..c322688629 100644 --- a/commands/proposal_submission.go +++ b/commands/proposal_submission.go @@ -37,19 +37,29 @@ import ( const ReferenceMaxLen int = 100 -var ( - validSources = map[protoTypes.AccountType]struct{}{ - protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, - protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD: {}, - protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: {}, - protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: {}, - } - validDestinations = map[protoTypes.AccountType]struct{}{ +var validTransfers = map[protoTypes.AccountType]map[protoTypes.AccountType]struct{}{ + protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: { protoTypes.AccountType_ACCOUNT_TYPE_GENERAL: {}, + protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: {}, + protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_PAID_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_LP_RECEIVED_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_RECEIVED_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MARKET_PROPOSERS: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_AVERAGE_POSITION: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RELATIVE_RETURN: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RETURN_VOLATILITY: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_VALIDATOR_RANKING: {}, + }, + protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: { + protoTypes.AccountType_ACCOUNT_TYPE_GENERAL: {}, + protoTypes.AccountType_ACCOUNT_TYPE_BOND: {}, + protoTypes.AccountType_ACCOUNT_TYPE_MARGIN: {}, + protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: {}, - protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: {}, + protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD: {}, protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_PAID_FEES: {}, protoTypes.AccountType_ACCOUNT_TYPE_REWARD_LP_RECEIVED_FEES: {}, protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_RECEIVED_FEES: {}, @@ -58,8 +68,24 @@ var ( protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RELATIVE_RETURN: {}, protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RETURN_VOLATILITY: {}, protoTypes.AccountType_ACCOUNT_TYPE_REWARD_VALIDATOR_RANKING: {}, - } -) + }, + protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: { + protoTypes.AccountType_ACCOUNT_TYPE_GENERAL: {}, + protoTypes.AccountType_ACCOUNT_TYPE_BOND: {}, + protoTypes.AccountType_ACCOUNT_TYPE_MARGIN: {}, + protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, + protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: {}, + protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_PAID_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_LP_RECEIVED_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MAKER_RECEIVED_FEES: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_MARKET_PROPOSERS: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_AVERAGE_POSITION: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RELATIVE_RETURN: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_RETURN_VOLATILITY: {}, + protoTypes.AccountType_ACCOUNT_TYPE_REWARD_VALIDATOR_RANKING: {}, + }, +} func CheckProposalSubmission(cmd *commandspb.ProposalSubmission) error { return checkProposalSubmission(cmd).ErrorOrNil() @@ -494,48 +520,47 @@ func checkNewTransferChanges(change *protoTypes.ProposalTerms_NewTransfer) Error if changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_UNSPECIFIED { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.source_type", ErrIsRequired) } - + validDest, ok := validTransfers[changes.SourceType] // source account type may be one of the following: - if _, ok := validSources[changes.SourceType]; !ok { + if !ok { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.source_type", ErrIsNotValid) } - if changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_UNSPECIFIED { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination_type", ErrIsRequired) } - if changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD && changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD { + if _, ok := validDest[changes.DestinationType]; !ok { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination_type", ErrIsNotValid) } + dest := changes.DestinationType - if changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY && changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY { - return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination_type", ErrIsNotValid) - } - - // destination account type may be one of the following: - if _, ok := validDestinations[changes.DestinationType]; !ok { - return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination_type", ErrIsNotValid) - } - - if changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL && !IsVegaPublicKey(changes.Destination) { + // party accounts: check pubkey + if (dest == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL || + dest == protoTypes.AccountType_ACCOUNT_TYPE_BOND || + dest == protoTypes.AccountType_ACCOUNT_TYPE_MARGIN) && + !IsVegaPublicKey(changes.Destination) { errs.AddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrShouldBeAValidVegaPublicKey) } - if (changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD || - changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY || - changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE) && - len(changes.Source) > 0 { + // insurance account type requires a source, other sources are global + if changes.SourceType == protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE { + if len(changes.Source) == 0 { + return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.source", ErrIsNotValid) + } + // destination == source + if dest == changes.SourceType && changes.Source == changes.Destination { + return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrIsNotValid) + } + } else if len(changes.Source) > 0 { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.source", ErrIsNotValid) } - if (changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD || - changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY || - changes.DestinationType == protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE) && - len(changes.Destination) > 0 { - return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrIsNotValid) - } - - if changes.SourceType == changes.DestinationType && changes.Source == changes.Destination { + // global destination accounts == no source + if (dest == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL || + dest == protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE || + dest == protoTypes.AccountType_ACCOUNT_TYPE_BOND || + dest == protoTypes.AccountType_ACCOUNT_TYPE_MARGIN) && + len(changes.Destination) == 0 { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrIsNotValid) } diff --git a/commands/proposal_submission_new_transfer_test.go b/commands/proposal_submission_new_transfer_test.go index c956307da7..7fa24d8da2 100644 --- a/commands/proposal_submission_new_transfer_test.go +++ b/commands/proposal_submission_new_transfer_test.go @@ -137,7 +137,7 @@ func testInvalidDestForMetric(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: tp, Destination: "", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -178,7 +178,7 @@ func testInvalidAssetForMetric(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: inv, Destination: "", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -205,7 +205,7 @@ func testInvalidAssetForMetric(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_REWARD_MAKER_RECEIVED_FEES, Destination: "", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -224,7 +224,7 @@ func testInvalidAssetForMetric(t *testing.T) { }, }) - require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.recurring.dispatch_strategy.asset_for_metric"), commands.ErrShouldBeAValidVegaID) + require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.recurring.dispatch_strategy.asset_for_metric"), commands.ErrShouldBeAValidVegaID, err.Error()) } func testRecurringWithDispatchInvalidTypes(t *testing.T) { @@ -240,6 +240,8 @@ func testRecurringWithDispatchInvalidTypes(t *testing.T) { delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_REWARD_RETURN_VOLATILITY) delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_REWARD_RELATIVE_RETURN) delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_REWARD_AVERAGE_POSITION) + delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_INSURANCE) + delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_GENERAL) delete(invalidTypes, types.AccountType_ACCOUNT_TYPE_UNSPECIFIED) @@ -251,7 +253,7 @@ func testRecurringWithDispatchInvalidTypes(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: inv, Destination: "", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -267,7 +269,7 @@ func testRecurringWithDispatchInvalidTypes(t *testing.T) { }, }, }) - require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.destination_type"), commands.ErrIsNotValid) + require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.destination_type"), commands.ErrIsNotValid, inv.String()) } } @@ -279,7 +281,7 @@ func testRecurringWithDestinationAndDispatch(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: "zohar", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -306,7 +308,7 @@ func testOneOffWithNegativeDeliverOn(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: "zohar", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -332,7 +334,7 @@ func testInvalidGeneralPubKey(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: "zohar", TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -352,14 +354,13 @@ func testInvalidGeneralPubKey(t *testing.T) { func testOneOffWithInvalidDestinationType(t *testing.T) { dests := []types.AccountType{ - types.AccountType_ACCOUNT_TYPE_REWARD_LP_RECEIVED_FEES, - types.AccountType_ACCOUNT_TYPE_REWARD_MAKER_PAID_FEES, - types.AccountType_ACCOUNT_TYPE_REWARD_MAKER_RECEIVED_FEES, - types.AccountType_ACCOUNT_TYPE_REWARD_MARKET_PROPOSERS, - types.AccountType_ACCOUNT_TYPE_REWARD_AVERAGE_POSITION, - types.AccountType_ACCOUNT_TYPE_REWARD_RELATIVE_RETURN, - types.AccountType_ACCOUNT_TYPE_REWARD_RETURN_VOLATILITY, - types.AccountType_ACCOUNT_TYPE_REWARD_VALIDATOR_RANKING, + types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, + types.AccountType_ACCOUNT_TYPE_SETTLEMENT, + types.AccountType_ACCOUNT_TYPE_MARGIN, + types.AccountType_ACCOUNT_TYPE_BOND, + types.AccountType_ACCOUNT_TYPE_FEES_MAKER, + types.AccountType_ACCOUNT_TYPE_FEES_INFRASTRUCTURE, + types.AccountType_ACCOUNT_TYPE_FEES_LIQUIDITY, } for _, dest := range dests { @@ -370,7 +371,7 @@ func testOneOffWithInvalidDestinationType(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: dest, TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, Asset: "abcde", @@ -395,8 +396,9 @@ func testNewRecurringGovernanceTransferInvalidEndEpoch(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, + Destination: crypto.RandomHash(), TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, Asset: "abcde", Kind: &types.NewTransferConfiguration_Recurring{ @@ -421,8 +423,9 @@ func testNewTransferWithNoKind(t *testing.T) { Changes: &types.NewTransferConfiguration{ FractionOfBalance: "0.5", Amount: "1000", - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, + Destination: crypto.RandomHash(), TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, Asset: "abcde", }, @@ -475,7 +478,7 @@ func testNewTransferChangeSubmissionWithoutDestinationTypeFails(t *testing.T) { Change: &types.ProposalTerms_NewTransfer{ NewTransfer: &types.NewTransfer{ Changes: &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, }, }, }, @@ -512,7 +515,7 @@ func testNewTransferChangeSubmissionInvalidDestinationTypeFails(t *testing.T) { Change: &types.ProposalTerms_NewTransfer{ NewTransfer: &types.NewTransfer{ Changes: &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType(at), }, }, @@ -546,7 +549,6 @@ func testNewTransferChangeSubmissionInvalidSourceTypeFails(t *testing.T) { } delete(allAccountTypes, int32(types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY)) delete(allAccountTypes, int32(types.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE)) - delete(allAccountTypes, int32(types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD)) delete(allAccountTypes, int32(types.AccountType_ACCOUNT_TYPE_INSURANCE)) delete(allAccountTypes, int32(types.AccountType_ACCOUNT_TYPE_UNSPECIFIED)) @@ -562,10 +564,10 @@ func testNewTransferChangeSubmissionInvalidSourceTypeFails(t *testing.T) { }, }, }) - require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.source_type"), commands.ErrIsNotValid) + require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.source_type"), commands.ErrIsNotValid, types.AccountType(at).String()) } - validSourceAccountTypes := []types.AccountType{types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, types.AccountType_ACCOUNT_TYPE_INSURANCE} + validSourceAccountTypes := []types.AccountType{types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, types.AccountType_ACCOUNT_TYPE_INSURANCE} for _, at := range validSourceAccountTypes { err := checkProposalSubmission(&commandspb.ProposalSubmission{ Terms: &types.ProposalTerms{ @@ -588,7 +590,7 @@ func testNewTransferChangeSubmissionInvalidSourceFails(t *testing.T) { Change: &types.ProposalTerms_NewTransfer{ NewTransfer: &types.NewTransfer{ Changes: &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, Source: "some source", DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, }, @@ -606,14 +608,15 @@ func testNewTransferChangeSubmissionInvalidDestinationFails(t *testing.T) { NewTransfer: &types.NewTransfer{ Changes: &types.NewTransferConfiguration{ SourceType: types.AccountType_ACCOUNT_TYPE_INSURANCE, - DestinationType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, - Destination: "some destination", + Source: "some market", + DestinationType: types.AccountType_ACCOUNT_TYPE_INSURANCE, + Destination: "", }, }, }, }, }) - require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.destination"), commands.ErrIsNotValid) + require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.destination"), commands.ErrIsNotValid, err.Error()) } func testCancelTransferChangeSubmission(t *testing.T) { @@ -675,7 +678,7 @@ func testNewTransferChangeSubmissionInvalidTransferTypeFails(t *testing.T) { Change: &types.ProposalTerms_NewTransfer{ NewTransfer: &types.NewTransfer{ Changes: &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: crypto.RandomHash(), TransferType: tp, @@ -694,7 +697,7 @@ func testNewTransferChangeSubmissionInvalidTransferTypeFails(t *testing.T) { func testNewTransferChangeSubmissionInvalidAmountFails(t *testing.T) { transfer := &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: crypto.RandomHash(), TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -759,7 +762,7 @@ func testNewTransferChangeSubmissionInvalidAmountFails(t *testing.T) { func testNewTransferChangeSubmissionInvalidAseetFails(t *testing.T) { transfer := &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: crypto.RandomHash(), TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, @@ -797,7 +800,7 @@ func testNewTransferChangeSubmissionInvalidFractionFails(t *testing.T) { } transfer := &types.NewTransferConfiguration{ - SourceType: types.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD, + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, Destination: crypto.RandomHash(), TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, From c7e8b258b3982e1b27e1939f414a5e827522ef9d Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Mon, 16 Oct 2023 15:20:31 +0100 Subject: [PATCH 2/3] fix: remove all but the general accounts Signed-off-by: Elias Van Ootegem --- commands/proposal_submission.go | 4 - .../proposal_submission_new_transfer_test.go | 73 +++++++++++++++++ core/integration/features/perpetual.feature | 80 +++++++++++++++++++ 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/commands/proposal_submission.go b/commands/proposal_submission.go index c322688629..30a175dfea 100644 --- a/commands/proposal_submission.go +++ b/commands/proposal_submission.go @@ -54,8 +54,6 @@ var validTransfers = map[protoTypes.AccountType]map[protoTypes.AccountType]struc }, protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: { protoTypes.AccountType_ACCOUNT_TYPE_GENERAL: {}, - protoTypes.AccountType_ACCOUNT_TYPE_BOND: {}, - protoTypes.AccountType_ACCOUNT_TYPE_MARGIN: {}, protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: {}, @@ -71,8 +69,6 @@ var validTransfers = map[protoTypes.AccountType]map[protoTypes.AccountType]struc }, protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_INSURANCE: { protoTypes.AccountType_ACCOUNT_TYPE_GENERAL: {}, - protoTypes.AccountType_ACCOUNT_TYPE_BOND: {}, - protoTypes.AccountType_ACCOUNT_TYPE_MARGIN: {}, protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE: {}, protoTypes.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY: {}, protoTypes.AccountType_ACCOUNT_TYPE_GLOBAL_REWARD: {}, diff --git a/commands/proposal_submission_new_transfer_test.go b/commands/proposal_submission_new_transfer_test.go index 7fa24d8da2..1d4c6e4a6e 100644 --- a/commands/proposal_submission_new_transfer_test.go +++ b/commands/proposal_submission_new_transfer_test.go @@ -18,6 +18,7 @@ package commands_test import ( "errors" "testing" + "time" "code.vegaprotocol.io/vega/commands" "code.vegaprotocol.io/vega/libs/crypto" @@ -50,6 +51,7 @@ func TestCheckProposalSubmissionForNewTransfer(t *testing.T) { t.Run("Submitting a new recurring transfer change with a dispatch strategy and incompatible empty asset for metric", testInvalidAssetForMetric) t.Run("Submitting a new recurring transfer change with a dispatch strategy and mismatching destination type for metric", testInvalidDestForMetric) t.Run("Submitting a new transfer change with destination type general and an invalid vega public key", testInvalidGeneralPubKey) + t.Run("Submitting a new transfer change with destination type general and an invalid vega public key", testOnlyGeneralValid) } func testInvalidDestForMetric(t *testing.T) { @@ -326,6 +328,77 @@ func testOneOffWithNegativeDeliverOn(t *testing.T) { require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.oneoff.deliveron"), commands.ErrMustBePositiveOrZero) } +func testOnlyGeneralValid(t *testing.T) { + partyAccs := []types.AccountType{ + types.AccountType_ACCOUNT_TYPE_MARGIN, + types.AccountType_ACCOUNT_TYPE_BOND, + } + // start with a valid transfer to the general account + prop := &commandspb.ProposalSubmission{ + Rationale: &types.ProposalRationale{ + Description: "valid", + Title: "test", + }, + Terms: &types.ProposalTerms{ + ClosingTimestamp: time.Now().Unix() + 100, + EnactmentTimestamp: time.Now().Unix() + 200, + Change: &types.ProposalTerms_NewTransfer{ + NewTransfer: &types.NewTransfer{ + Changes: &types.NewTransferConfiguration{ + FractionOfBalance: "0.5", + Amount: "1000", + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, + DestinationType: types.AccountType_ACCOUNT_TYPE_GENERAL, + Destination: crypto.RandomHash(), + TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, + Asset: "abcde", + Kind: &types.NewTransferConfiguration_OneOff{ + OneOff: &types.OneOffTransfer{ + DeliverOn: 0, + }, + }, + }, + }, + }, + }, + } + err := checkProposalSubmission(prop) + require.True(t, err.Empty()) + // none of the other accounts are valid destination types: + for _, at := range partyAccs { + prop := &commandspb.ProposalSubmission{ + Rationale: &types.ProposalRationale{ + Description: "invalid", + Title: "test", + }, + Terms: &types.ProposalTerms{ + ClosingTimestamp: time.Now().Unix() + 100, + EnactmentTimestamp: time.Now().Unix() + 200, + Change: &types.ProposalTerms_NewTransfer{ + NewTransfer: &types.NewTransfer{ + Changes: &types.NewTransferConfiguration{ + FractionOfBalance: "0.5", + Amount: "1000", + SourceType: types.AccountType_ACCOUNT_TYPE_NETWORK_TREASURY, + DestinationType: at, + Destination: crypto.RandomHash(), // ensure a valid hash + TransferType: types.GovernanceTransferType_GOVERNANCE_TRANSFER_TYPE_ALL_OR_NOTHING, + Asset: "abcde", + Kind: &types.NewTransferConfiguration_OneOff{ + OneOff: &types.OneOffTransfer{ + DeliverOn: 0, + }, + }, + }, + }, + }, + }, + } + err = checkProposalSubmission(prop) + require.Contains(t, err.Get("proposal_submission.terms.change.new_transfer.changes.destination_type"), commands.ErrIsNotValid) + } +} + func testInvalidGeneralPubKey(t *testing.T) { err := checkProposalSubmission(&commandspb.ProposalSubmission{ Terms: &types.ProposalTerms{ diff --git a/core/integration/features/perpetual.feature b/core/integration/features/perpetual.feature index 73d709e639..0f304d6b54 100644 --- a/core/integration/features/perpetual.feature +++ b/core/integration/features/perpetual.feature @@ -122,3 +122,83 @@ Feature: Simple test creating a perpetual market. | market id | state | settlement price | | ETH/DEC19 | MARKET_STATE_UPDATE_TYPE_TERMINATE | 976 | Then the market state should be "STATE_CLOSED" for the market "ETH/DEC19" + + @PerpetualCancel + Scenario: 003 Cancel a perps market in opening auction + # the amount ought to be 390,500.000,000,000,000,000,000 + Given the parties submit the following liquidity provision: + | id | party | market id | commitment amount | fee | lp type | + | lp1 | lpprov | ETH/DEC19 | 3905000000000000 | 0.3 | submission | + And the parties place the following pegged iceberg orders: + | party | market id | peak size | minimum visible size | side | pegged reference | volume | offset | reference | + | lpprov | ETH/DEC19 | 4000000000000000 | 3905000000000000 | buy | BID | 4000000000000000 | 1 | lp-ice-buy | + | lpprov | ETH/DEC19 | 4000000000000000 | 3905000000000000 | sell | ASK | 4000000000000000 | 1 | lp-ice-sell | + And the parties place the following orders: + | party | market id | side | volume | price | resulting trades | type | tif | reference | + | trader1 | ETH/DEC19 | buy | 5 | 1001 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-1 | + | trader1 | ETH/DEC19 | buy | 5 | 900 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-2 | + | trader1 | ETH/DEC19 | buy | 1 | 100 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-3 | + | trader2 | ETH/DEC19 | sell | 5 | 1200 | 0 | TYPE_LIMIT | TIF_GTC | t2-s-1 | + | trader2 | ETH/DEC19 | sell | 1 | 100000 | 0 | TYPE_LIMIT | TIF_GTC | t2-s-2 | + When the opening auction period ends for market "ETH/DEC19" + Then the market data for the market "ETH/DEC19" should be: + | mark price | trading mode | auction trigger | + | 0 | TRADING_MODE_OPENING_AUCTION | AUCTION_TRIGGER_OPENING | + And the parties should have the following account balances: + | party | asset | market id | margin | general | + | trader1 | ETH | ETH/DEC19 | 60659552186 | 9999999999999939340447814 | + + + # example of how to use the oracle + When the oracles broadcast data with block time signed with "0xCAFECAFE1": + | name | value | time offset | + | perp.ETH.value | 975 | -2s | + | perp.ETH.value | 977 | -1s | + + And the parties place the following orders with ticks: + | party | market id | side | volume | price | resulting trades | type | tif | reference | + | trader2 | ETH/DEC19 | sell | 1 | 951 | 0 | TYPE_LIMIT | TIF_GTC | t2-s-2 | + Then the market data for the market "ETH/DEC19" should be: + | mark price | trading mode | auction trigger | + | 976 | TRADING_MODE_CONTINUOUS | AUCTION_TRIGGER_UNSPECIFIED | + When the market states are updated through governance: + | market id | state | settlement price | + | ETH/DEC19 | MARKET_STATE_UPDATE_TYPE_TERMINATE | 976 | + Then the market state should be "STATE_CLOSED" for the market "ETH/DEC19" + + @PerpetualCancel + Scenario: 003 Cancel a perps market in opening auction + # the amount ought to be 390,500.000,000,000,000,000,000 + Given the parties submit the following liquidity provision: + | id | party | market id | commitment amount | fee | lp type | + | lp1 | lpprov | ETH/DEC19 | 3905000000000000 | 0.3 | submission | + And the parties place the following pegged iceberg orders: + | party | market id | peak size | minimum visible size | side | pegged reference | volume | offset | reference | + | lpprov | ETH/DEC19 | 4000000000000000 | 3905000000000000 | buy | BID | 4000000000000000 | 1 | lp-ice-buy | + | lpprov | ETH/DEC19 | 4000000000000000 | 3905000000000000 | sell | ASK | 4000000000000000 | 1 | lp-ice-sell | + And the parties place the following orders: + | party | market id | side | volume | price | resulting trades | type | tif | reference | + | trader1 | ETH/DEC19 | buy | 5 | 1001 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-1 | + | trader1 | ETH/DEC19 | buy | 5 | 900 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-2 | + | trader1 | ETH/DEC19 | buy | 1 | 100 | 0 | TYPE_LIMIT | TIF_GTC | t1-b-3 | + | trader2 | ETH/DEC19 | sell | 5 | 1200 | 0 | TYPE_LIMIT | TIF_GTC | t2-s-1 | + | trader2 | ETH/DEC19 | sell | 1 | 100000 | 0 | TYPE_LIMIT | TIF_GTC | t2-s-2 | + When the opening auction period ends for market "ETH/DEC19" + Then the market data for the market "ETH/DEC19" should be: + | mark price | trading mode | auction trigger | + | 0 | TRADING_MODE_OPENING_AUCTION | AUCTION_TRIGGER_OPENING | + And the parties should have the following account balances: + | party | asset | market id | margin | general | + | trader1 | ETH | ETH/DEC19 | 60659552186 | 9999999999999939340447814 | + + + # example of how to use the oracle + When the oracles broadcast data with block time signed with "0xCAFECAFE1": + | name | value | time offset | + | perp.ETH.value | 975 | -2s | + | perp.ETH.value | 977 | -1s | + + And the market states are updated through governance: + | market id | state | settlement price | + | ETH/DEC19 | MARKET_STATE_UPDATE_TYPE_TERMINATE | 976 | + Then the market state should be "STATE_CANCELLED" for the market "ETH/DEC19" From 81f5cec8c6cf36dcf03ab92d3febb676e8830f9f Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Mon, 16 Oct 2023 15:47:33 +0100 Subject: [PATCH 3/3] fix: code review comments Signed-off-by: Elias Van Ootegem --- commands/proposal_submission.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/commands/proposal_submission.go b/commands/proposal_submission.go index 30a175dfea..3207e24e40 100644 --- a/commands/proposal_submission.go +++ b/commands/proposal_submission.go @@ -531,10 +531,7 @@ func checkNewTransferChanges(change *protoTypes.ProposalTerms_NewTransfer) Error dest := changes.DestinationType // party accounts: check pubkey - if (dest == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL || - dest == protoTypes.AccountType_ACCOUNT_TYPE_BOND || - dest == protoTypes.AccountType_ACCOUNT_TYPE_MARGIN) && - !IsVegaPublicKey(changes.Destination) { + if dest == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL && !IsVegaPublicKey(changes.Destination) { errs.AddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrShouldBeAValidVegaPublicKey) } @@ -553,9 +550,7 @@ func checkNewTransferChanges(change *protoTypes.ProposalTerms_NewTransfer) Error // global destination accounts == no source if (dest == protoTypes.AccountType_ACCOUNT_TYPE_GENERAL || - dest == protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE || - dest == protoTypes.AccountType_ACCOUNT_TYPE_BOND || - dest == protoTypes.AccountType_ACCOUNT_TYPE_MARGIN) && + dest == protoTypes.AccountType_ACCOUNT_TYPE_INSURANCE) && len(changes.Destination) == 0 { return errs.FinalAddForProperty("proposal_submission.terms.change.new_transfer.changes.destination", ErrIsNotValid) }