more finishProposalsTx checks and corresponding tests

vms/platformvm/txs/builder/camino_builder.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ava-labs/avalanchego/chains/atomic"
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/snow"
+	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
 	"github.com/ava-labs/avalanchego/utils/timer/mockable"
 	"github.com/ava-labs/avalanchego/vms/components/avax"
@@ -733,6 +734,11 @@ func (b *caminoBuilder) FinishProposalsTx(
 		}
 	}
 
+	utils.Sort(utx.EarlyFinishedSuccessfulProposalIDs)
+	utils.Sort(utx.EarlyFinishedFailedProposalIDs)
+	utils.Sort(utx.ExpiredSuccessfulProposalIDs)
+	utils.Sort(utx.ExpiredFailedProposalIDs)
+
 	tx, err := txs.NewSigned(utx, txs.Codec, nil)
 	if err != nil {
 		return nil, err

vms/platformvm/txs/camino_finish_proposals_tx.go

@@ -9,15 +9,17 @@ import (
 
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/snow"
+	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/set"
 	"github.com/ava-labs/avalanchego/vms/platformvm/locked"
 )
 
 var (
 	_ UnsignedTx = (*FinishProposalsTx)(nil)
 
-	errNoFinishedProposals = errors.New("no expired or successful proposals")
-	errNotUniqueProposalID = errors.New("not unique proposal id")
+	errNoFinishedProposals          = errors.New("no expired or successful proposals")
+	errNotUniqueProposalID          = errors.New("not unique proposal id")
+	errNotSortedOrUniqueProposalIDs = errors.New("not sorted or not unique proposal ids")
 )
 
 // FinishProposalsTx is an unsigned removeExpiredProposalsTx
@@ -46,6 +48,11 @@ func (tx *FinishProposalsTx) SyntacticVerify(ctx *snow.Context) error {
 		len(tx.ExpiredSuccessfulProposalIDs) == 0 &&
 		len(tx.ExpiredFailedProposalIDs) == 0:
 		return errNoFinishedProposals
+	case !utils.IsSortedAndUniqueSortable(tx.EarlyFinishedSuccessfulProposalIDs) ||
+		!utils.IsSortedAndUniqueSortable(tx.EarlyFinishedFailedProposalIDs) ||
+		!utils.IsSortedAndUniqueSortable(tx.ExpiredSuccessfulProposalIDs) ||
+		!utils.IsSortedAndUniqueSortable(tx.ExpiredFailedProposalIDs):
+		return errNotSortedOrUniqueProposalIDs
 	}
 
 	if err := tx.BaseTx.SyntacticVerify(ctx); err != nil {
@@ -56,9 +63,6 @@ func (tx *FinishProposalsTx) SyntacticVerify(ctx *snow.Context) error {
 		len(tx.ExpiredSuccessfulProposalIDs) + len(tx.ExpiredFailedProposalIDs)
 	uniqueProposals := set.NewSet[ids.ID](totalProposalsCount)
 	for _, proposalID := range tx.EarlyFinishedSuccessfulProposalIDs {
-		if uniqueProposals.Contains(proposalID) {
-			return errNotUniqueProposalID
-		}
 		uniqueProposals.Add(proposalID)
 	}
 	for _, proposalID := range tx.EarlyFinishedFailedProposalIDs {
@@ -92,3 +96,10 @@ func (tx *FinishProposalsTx) SyntacticVerify(ctx *snow.Context) error {
 func (tx *FinishProposalsTx) Visit(visitor Visitor) error {
 	return visitor.FinishProposalsTx(tx)
 }
+
+func (tx *FinishProposalsTx) ProposalIDs() []ids.ID {
+	lockTxIDs := tx.EarlyFinishedSuccessfulProposalIDs
+	lockTxIDs = append(lockTxIDs, tx.EarlyFinishedFailedProposalIDs...)
+	lockTxIDs = append(lockTxIDs, tx.ExpiredSuccessfulProposalIDs...)
+	return append(lockTxIDs, tx.ExpiredFailedProposalIDs...)
+}

vms/platformvm/txs/camino_finish_proposals_tx_test.go

@@ -44,34 +44,61 @@ func TestFinishProposalsTxSyntacticVerify(t *testing.T) {
 			},
 			expectedErr: errNoFinishedProposals,
 		},
-		// TODO@ unique
+		"Not sorted proposals in EarlyFinishedSuccessfulProposalIDs": {
+			tx: &FinishProposalsTx{
+				BaseTx:                             baseTx,
+				EarlyFinishedSuccessfulProposalIDs: []ids.ID{proposalID2, proposalID1},
+			},
+			expectedErr: errNotSortedOrUniqueProposalIDs,
+		},
+		"Not sorted proposals in EarlyFinishedFailedProposalIDs": {
+			tx: &FinishProposalsTx{
+				BaseTx:                         baseTx,
+				EarlyFinishedFailedProposalIDs: []ids.ID{proposalID2, proposalID1},
+			},
+			expectedErr: errNotSortedOrUniqueProposalIDs,
+		},
+		"Not sorted proposals in ExpiredSuccessfulProposalIDs": {
+			tx: &FinishProposalsTx{
+				BaseTx:                       baseTx,
+				ExpiredSuccessfulProposalIDs: []ids.ID{proposalID2, proposalID1},
+			},
+			expectedErr: errNotSortedOrUniqueProposalIDs,
+		},
+		"Not sorted proposals in ExpiredFailedProposalIDs": {
+			tx: &FinishProposalsTx{
+				BaseTx:                   baseTx,
+				ExpiredFailedProposalIDs: []ids.ID{proposalID2, proposalID1},
+			},
+			expectedErr: errNotSortedOrUniqueProposalIDs,
+		},
 		"Not unique proposals in EarlyFinishedSuccessfulProposalIDs": {
 			tx: &FinishProposalsTx{
 				BaseTx:                             baseTx,
 				EarlyFinishedSuccessfulProposalIDs: []ids.ID{proposalID1, proposalID1},
 			},
-			expectedErr: errNotUniqueProposalID,
+			expectedErr: errNotSortedOrUniqueProposalIDs,
 		},
 		"Not unique proposals in EarlyFinishedFailedProposalIDs": {
 			tx: &FinishProposalsTx{
 				BaseTx:                         baseTx,
 				EarlyFinishedFailedProposalIDs: []ids.ID{proposalID1, proposalID1},
 			},
-			expectedErr: errNotUniqueProposalID,
+			expectedErr: errNotSortedOrUniqueProposalIDs,
 		},
 		"Not unique proposals in ExpiredSuccessfulProposalIDs": {
 			tx: &FinishProposalsTx{
 				BaseTx:                       baseTx,
 				ExpiredSuccessfulProposalIDs: []ids.ID{proposalID1, proposalID1},
 			},
-			expectedErr: errNotUniqueProposalID,
+			expectedErr: errNotSortedOrUniqueProposalIDs,
 		},
 		"Not unique proposals in ExpiredFailedProposalIDs": {
 			tx: &FinishProposalsTx{
 				BaseTx:                   baseTx,
 				ExpiredFailedProposalIDs: []ids.ID{proposalID1, proposalID1},
 			},
-			expectedErr: errNotUniqueProposalID,
+			expectedErr: errNotSortedOrUniqueProposalIDs,
 		},
 		"Not unique proposals in EarlyFinishedSuccessfulProposalIDs and EarlyFinishedFailedProposalIDs": {
 			tx: &FinishProposalsTx{

vms/platformvm/txs/executor/camino_helpers_test.go

@@ -377,6 +377,34 @@ func generateTestUTXOWithIndex(txID ids.ID, outIndex uint32, assetID ids.ID, amo
 	return testUTXO
 }
 
+func generateTestOutFromUTXO(utxo *avax.UTXO, depositTxID, bondTxID ids.ID) *avax.TransferableOutput {
+	out := utxo.Out
+	if lockedOut, ok := out.(*locked.Out); ok {
+		out = lockedOut.TransferableOut
+	}
+	secpOut, ok := out.(*secp256k1fx.TransferOutput)
+	if !ok {
+		panic("not secp out")
+	}
+	var innerOut avax.TransferableOut = &secp256k1fx.TransferOutput{
+		Amt:          secpOut.Amt,
+		OutputOwners: secpOut.OutputOwners,
+	}
+	if depositTxID != ids.Empty || bondTxID != ids.Empty {
+		innerOut = &locked.Out{
+			IDs: locked.IDs{
+				DepositTxID: depositTxID,
+				BondTxID:    bondTxID,
+			},
+			TransferableOut: innerOut,
+		}
+	}
+	return &avax.TransferableOutput{
+		Asset: avax.Asset{ID: utxo.AssetID()},
+		Out:   innerOut,
+	}
+}
+
 func generateTestOut(assetID ids.ID, amount uint64, outputOwners secp256k1fx.OutputOwners, depositTxID, bondTxID ids.ID) *avax.TransferableOutput {
 	var out avax.TransferableOut = &secp256k1fx.TransferOutput{
 		Amt:          amount,

vms/platformvm/txs/executor/camino_tx_executor.go

@@ -87,6 +87,15 @@ var (
 	errProposerCredentialMismatch        = errors.New("proposer credential isn't matching")
 	errWrongProposalBondAmount           = errors.New("wrong proposal bond amount")
 	errVoterCredentialMismatch           = errors.New("voter credential isn't matching")
+	errNotSuccessfulProposal             = errors.New("proposal is not successful")
+	errSuccessfulProposal                = errors.New("proposal is successful")
+	errNotEarlyFinishedProposal          = errors.New("proposal is not early finished")
+	errEarlyFinishedProposal             = errors.New("proposal is early finished")
+	errNotExpiredProposal                = errors.New("proposal is not expired")
+	errExpiredProposal                   = errors.New("proposal is expired")
+	errProposalsAreNotExpiredYet         = errors.New("proposals are not expired yet")
+	errEarlyFinishedProposalsMismatch    = errors.New("early proposals mismatch")
+	errExpiredProposalsMismatch          = errors.New("expired proposals mismatch")
 )
 
 type CaminoStandardTxExecutor struct {
@@ -1907,20 +1916,39 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 		return err
 	}
 
-	chainTime := e.State.GetTimestamp()
+	// basic checks
 
-	if !e.Config.IsDACPhaseActivated(chainTime) {
-		return errNotDACPhase
+	chainTime := e.State.GetTimestamp()
+	nextToExpireProposalIDs, expirationTime, err := e.State.GetNextToExpireProposalIDsAndTime(nil)
+	if err != nil {
+		return err
 	}
+	proposalIDsToFinish, err := e.State.GetProposalIDsToFinish()
+	if err != nil {
+		return err
+	}
+	isExpirationTime := expirationTime.Equal(chainTime)
 
-	if len(e.Tx.Creds) != 0 {
+	// TODO@ if chainTime == expirationTime, then all expired proposals must be in tx
+
+	switch {
+	case len(e.Tx.Creds) != 0:
 		return errWrongCredentialsNumber
+	case !e.Config.IsDACPhaseActivated(chainTime):
+		return errNotDACPhase
+	case !isExpirationTime &&
+		len(tx.ExpiredSuccessfulProposalIDs)+len(tx.ExpiredFailedProposalIDs) != 0:
+		return errProposalsAreNotExpiredYet
+	case isExpirationTime &&
+		len(tx.ExpiredSuccessfulProposalIDs)+len(tx.ExpiredFailedProposalIDs) != len(nextToExpireProposalIDs):
+		return errExpiredProposalsMismatch
+	case len(tx.EarlyFinishedSuccessfulProposalIDs)+len(tx.EarlyFinishedFailedProposalIDs) != len(proposalIDsToFinish):
+		return errEarlyFinishedProposalsMismatch
 	}
 
-	lockTxIDs := append(tx.EarlyFinishedSuccessfulProposalIDs, tx.EarlyFinishedFailedProposalIDs...) //nolint:gocritic
-	lockTxIDs = append(lockTxIDs, tx.ExpiredSuccessfulProposalIDs...)
-	lockTxIDs = append(lockTxIDs, tx.ExpiredFailedProposalIDs...)
-	expectedIns, expectedOuts, err := e.FlowChecker.Unlock(e.State, lockTxIDs, locked.StateBonded)
+	// verify ins and outs
+
+	expectedIns, expectedOuts, err := e.FlowChecker.Unlock(e.State, tx.ProposalIDs(), locked.StateBonded)
 	if err != nil {
 		return err
 	}
@@ -1935,28 +1963,75 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 		return fmt.Errorf("%w: invalid outputs", errInvalidSystemTxBody)
 	}
 
+	// getting early finished and expired proposal IDs to check that they match tx proposal IDs
+
+	proposalIDsToFinishSet := set.NewSet[ids.ID](len(proposalIDsToFinish))
+	for _, proposalID := range proposalIDsToFinish {
+		proposalIDsToFinishSet.Add(proposalID)
+	}
+
+	nextToExpireProposalIDsSet := set.NewSet[ids.ID](len(nextToExpireProposalIDs))
+	if isExpirationTime {
+		for _, proposalID := range nextToExpireProposalIDs {
+			nextToExpireProposalIDsSet.Add(proposalID)
+		}
+	}
+
+	// processing tx proposal IDs
+
+	// TODO@ what if early finished proposal expires at the finishTx block?
+	// TODO@ meaning that its id will be both in expired and early finished
+	// TODO@ prevent failing
+
 	for _, proposalID := range tx.EarlyFinishedSuccessfulProposalIDs {
 		proposal, err := e.State.GetProposal(proposalID)
 		if err != nil {
 			return err
 		}
 
+		if !proposal.IsSuccessful() {
+			return errNotSuccessfulProposal
+		}
+
+		if !proposalIDsToFinishSet.Contains(proposalID) {
+			return errNotEarlyFinishedProposal
+		}
+
+		if nextToExpireProposalIDsSet.Contains(proposalID) {
+			return errExpiredProposal
+		}
+
 		// try to execute proposal
 		if err := proposal.Visit(e.proposalExecutor()); err != nil {
 			return err
 		}
 
 		e.State.RemoveProposal(proposalID, proposal)
 		e.State.RemoveProposalIDToFinish(proposalID)
+		proposalIDsToFinishSet.Remove(proposalID)
 	}
 
 	for _, proposalID := range tx.EarlyFinishedFailedProposalIDs {
 		proposal, err := e.State.GetProposal(proposalID)
 		if err != nil {
 			return err
 		}
+
+		if proposal.IsSuccessful() {
+			return errSuccessfulProposal
+		}
+
+		if !proposalIDsToFinishSet.Contains(proposalID) {
+			return errNotEarlyFinishedProposal
+		}
+
+		if nextToExpireProposalIDsSet.Contains(proposalID) {
+			return errExpiredProposal
+		}
+
 		e.State.RemoveProposal(proposalID, proposal)
 		e.State.RemoveProposalIDToFinish(proposalID)
+		proposalIDsToFinishSet.Remove(proposalID)
 	}
 
 	for _, proposalID := range tx.ExpiredSuccessfulProposalIDs {
@@ -1965,20 +2040,47 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 			return err
 		}
 
+		if !proposal.IsSuccessful() {
+			return errNotSuccessfulProposal
+		}
+
+		if proposalIDsToFinishSet.Contains(proposalID) {
+			return errEarlyFinishedProposal
+		}
+
+		if !nextToExpireProposalIDsSet.Contains(proposalID) {
+			return errNotExpiredProposal
+		}
+
 		// try to execute proposal
 		if err := proposal.Visit(e.proposalExecutor()); err != nil {
 			return err
 		}
 
 		e.State.RemoveProposal(proposalID, proposal)
+		nextToExpireProposalIDsSet.Remove(proposalID)
 	}
 
 	for _, proposalID := range tx.ExpiredFailedProposalIDs {
 		proposal, err := e.State.GetProposal(proposalID)
 		if err != nil {
 			return err
 		}
+
+		if proposal.IsSuccessful() {
+			return errSuccessfulProposal
+		}
+
+		if proposalIDsToFinishSet.Contains(proposalID) {
+			return errEarlyFinishedProposal
+		}
+
+		if !nextToExpireProposalIDsSet.Contains(proposalID) {
+			return errNotExpiredProposal
+		}
+
 		e.State.RemoveProposal(proposalID, proposal)
+		nextToExpireProposalIDsSet.Remove(proposalID)
 	}
 
 	avax.Consume(e.State, tx.Ins)