review fixes

vms/platformvm/dac/camino_add_member_proposal.go

@@ -78,8 +78,8 @@ func (p *AddMemberProposal) CreateFinishedProposalState(optionIndex uint32) (Pro
 	return proposalState, nil
 }
 
-func (p *AddMemberProposal) Visit(visitor VerifierVisitor) error {
-	return visitor.AddMemberProposal(p)
+func (p *AddMemberProposal) VerifyWith(verifier Verifier) error {
+	return verifier.AddMemberProposal(p)
 }
 
 type AddMemberProposalState struct {
@@ -192,10 +192,10 @@ func (p *AddMemberProposalState) ForceAddVote(voteIntf Vote) (ProposalState, err
 	return updatedProposal, nil
 }
 
-func (p *AddMemberProposalState) Visit(visitor ExecutorVisitor) error {
-	return visitor.AddMemberProposal(p)
+func (p *AddMemberProposalState) ExecuteWith(executor Executor) error {
+	return executor.AddMemberProposal(p)
 }
 
-func (p *AddMemberProposalState) GetBondTxIDs(visitor BondTxIDsGetter) ([]ids.ID, error) {
-	return visitor.AddMemberProposal(p)
+func (p *AddMemberProposalState) GetBondTxIDsWith(bondTxIDsGetter BondTxIDsGetter) ([]ids.ID, error) {
+	return bondTxIDsGetter.AddMemberProposal(p)
 }

vms/platformvm/dac/camino_base_fee_proposal.go

@@ -86,8 +86,8 @@ func (p *BaseFeeProposal) CreateFinishedProposalState(optionIndex uint32) (Propo
 	return proposalState, nil
 }
 
-func (p *BaseFeeProposal) Visit(visitor VerifierVisitor) error {
-	return visitor.BaseFeeProposal(p)
+func (p *BaseFeeProposal) VerifyWith(verifier Verifier) error {
+	return verifier.BaseFeeProposal(p)
 }
 
 type BaseFeeProposalState struct {
@@ -201,10 +201,10 @@ func (p *BaseFeeProposalState) ForceAddVote(voteIntf Vote) (ProposalState, error
 	return updatedProposal, nil
 }
 
-func (p *BaseFeeProposalState) Visit(visitor ExecutorVisitor) error {
-	return visitor.BaseFeeProposal(p)
+func (p *BaseFeeProposalState) ExecuteWith(executor Executor) error {
+	return executor.BaseFeeProposal(p)
 }
 
-func (p *BaseFeeProposalState) GetBondTxIDs(visitor BondTxIDsGetter) ([]ids.ID, error) {
-	return visitor.BaseFeeProposal(p)
+func (p *BaseFeeProposalState) GetBondTxIDsWith(bondTxIDsGetter BondTxIDsGetter) ([]ids.ID, error) {
+	return bondTxIDsGetter.BaseFeeProposal(p)
 }

vms/platformvm/dac/camino_exclude_member_proposal.go

@@ -87,8 +87,8 @@ func (p *ExcludeMemberProposal) CreateFinishedProposalState(optionIndex uint32)
 	return proposalState, nil
 }
 
-func (p *ExcludeMemberProposal) Visit(visitor VerifierVisitor) error {
-	return visitor.ExcludeMemberProposal(p)
+func (p *ExcludeMemberProposal) VerifyWith(verifier Verifier) error {
+	return verifier.ExcludeMemberProposal(p)
 }
 
 type ExcludeMemberProposalState struct {
@@ -201,10 +201,10 @@ func (p *ExcludeMemberProposalState) ForceAddVote(voteIntf Vote) (ProposalState,
 	return updatedProposal, nil
 }
 
-func (p *ExcludeMemberProposalState) Visit(visitor ExecutorVisitor) error {
-	return visitor.ExcludeMemberProposal(p)
+func (p *ExcludeMemberProposalState) ExecuteWith(executor Executor) error {
+	return executor.ExcludeMemberProposal(p)
 }
 
-func (p *ExcludeMemberProposalState) GetBondTxIDs(visitor BondTxIDsGetter) ([]ids.ID, error) {
-	return visitor.ExcludeMemberProposal(p)
+func (p *ExcludeMemberProposalState) GetBondTxIDsWith(bondTxIDsGetter BondTxIDsGetter) ([]ids.ID, error) {
+	return bondTxIDsGetter.ExcludeMemberProposal(p)
 }

vms/platformvm/dac/camino_proposal.go

@@ -22,13 +22,13 @@ var (
 	_ Proposal = (*AdminProposal)(nil)
 )
 
-type VerifierVisitor interface {
+type Verifier interface {
 	BaseFeeProposal(*BaseFeeProposal) error
 	AddMemberProposal(*AddMemberProposal) error
 	ExcludeMemberProposal(*ExcludeMemberProposal) error
 }
 
-type ExecutorVisitor interface {
+type Executor interface {
 	BaseFeeProposal(*BaseFeeProposalState) error
 	AddMemberProposal(*AddMemberProposalState) error
 	ExcludeMemberProposal(*ExcludeMemberProposalState) error
@@ -49,7 +49,7 @@ type Proposal interface {
 	AdminProposer() as.AddressState
 	CreateProposalState(allowedVoters []ids.ShortID) ProposalState
 	CreateFinishedProposalState(optionIndex uint32) (ProposalState, error)
-	Visit(VerifierVisitor) error
+	VerifyWith(Verifier) error
 
 	// Returns proposal options. (used in magellan)
 	//
@@ -69,9 +69,9 @@ type ProposalState interface {
 	CanBeFinished() bool
 	IsSuccessful() bool // should be called only for finished proposals
 	Outcome() any       // should be called only for finished successful proposals
-	Visit(ExecutorVisitor) error
+	ExecuteWith(Executor) error
 	// Visits getter and returns additional lock tx ids, that should be unbonded when this proposal is successfully finished.
-	GetBondTxIDs(BondTxIDsGetter) ([]ids.ID, error)
+	GetBondTxIDsWith(BondTxIDsGetter) ([]ids.ID, error)
 	// Will return modified ProposalState with added vote, original ProposalState will not be modified!
 	AddVote(voterAddress ids.ShortID, vote Vote) (ProposalState, error)
 

vms/platformvm/txs/builder/camino_builder.go

@@ -702,46 +702,40 @@ func (b *caminoBuilder) FinishProposalsTx(
 		BlockchainID: b.ctx.ChainID,
 	}}}
 
-	lockTxIDs := append(earlyFinishedProposalIDs, expiredProposalIDs...) //nolint:gocritic
-
-	for _, proposalID := range earlyFinishedProposalIDs {
-		proposal, err := state.GetProposal(proposalID)
-		if err != nil {
-			return nil, fmt.Errorf("couldn't get proposal from state: %w", err)
-		}
-		if proposal.IsSuccessful() {
-			bondTxIDs, err := proposal.GetBondTxIDs(dac.NewProposalBondTxIDsGetter(state))
+	var err error
+	sortOutProposals := func(proposalIDs []ids.ID) (successfulProposalIDs []ids.ID, failedProposalIDs []ids.ID, err error) {
+		for _, proposalID := range proposalIDs {
+			proposal, err := state.GetProposal(proposalID)
 			if err != nil {
-				return nil, err
+				return nil, nil, fmt.Errorf("couldn't get proposal from state: %w", err)
 			}
-			lockTxIDs = append(lockTxIDs, bondTxIDs...)
-			utx.EarlyFinishedSuccessfulProposalIDs = append(utx.EarlyFinishedSuccessfulProposalIDs, proposalID)
-		} else {
-			utx.EarlyFinishedFailedProposalIDs = append(utx.EarlyFinishedFailedProposalIDs, proposalID)
-		}
-	}
-	for _, proposalID := range expiredProposalIDs {
-		proposal, err := state.GetProposal(proposalID)
-		if err != nil {
-			return nil, fmt.Errorf("couldn't get proposal from state: %w", err)
-		}
-		if proposal.IsSuccessful() {
-			bondTxIDs, err := proposal.GetBondTxIDs(dac.NewProposalBondTxIDsGetter(state))
-			if err != nil {
-				return nil, err
+			if proposal.IsSuccessful() {
+				successfulProposalIDs = append(successfulProposalIDs, proposalID)
+			} else {
+				failedProposalIDs = append(failedProposalIDs, proposalID)
 			}
-			lockTxIDs = append(lockTxIDs, bondTxIDs...)
-			utx.ExpiredSuccessfulProposalIDs = append(utx.ExpiredSuccessfulProposalIDs, proposalID)
-		} else {
-			utx.ExpiredFailedProposalIDs = append(utx.ExpiredFailedProposalIDs, proposalID)
 		}
+		return successfulProposalIDs, failedProposalIDs, nil
+	}
+	utx.EarlyFinishedSuccessfulProposalIDs, utx.EarlyFinishedFailedProposalIDs, err = sortOutProposals(earlyFinishedProposalIDs)
+	if err != nil {
+		return nil, err
+	}
+	utx.ExpiredSuccessfulProposalIDs, utx.ExpiredFailedProposalIDs, err = sortOutProposals(expiredProposalIDs)
+	if err != nil {
+		return nil, err
 	}
 
 	utils.Sort(utx.EarlyFinishedSuccessfulProposalIDs)
 	utils.Sort(utx.EarlyFinishedFailedProposalIDs)
 	utils.Sort(utx.ExpiredSuccessfulProposalIDs)
 	utils.Sort(utx.ExpiredFailedProposalIDs)
 
+	lockTxIDs, err := dac.GetBondTxIDs(state, utx)
+	if err != nil {
+		return nil, err
+	}
+
 	ins, outs, err := b.Unlock(
 		state,
 		lockTxIDs,

vms/platformvm/txs/camino_finish_proposals_tx.go

@@ -103,3 +103,7 @@ func (tx *FinishProposalsTx) ProposalIDs() []ids.ID {
 	lockTxIDs = append(lockTxIDs, tx.ExpiredSuccessfulProposalIDs...)
 	return append(lockTxIDs, tx.ExpiredFailedProposalIDs...)
 }
+
+func (tx *FinishProposalsTx) SuccessfulProposalIDs() []ids.ID {
+	return append(tx.EarlyFinishedSuccessfulProposalIDs, tx.ExpiredSuccessfulProposalIDs...)
+}

vms/platformvm/txs/executor/camino_tx_executor.go

@@ -1765,7 +1765,7 @@ func (e *CaminoStandardTxExecutor) AddProposalTx(tx *txs.AddProposalTx) error {
 		return fmt.Errorf("%w: %s", errProposerCredentialMismatch, err)
 	}
 
-	if err := txProposal.Visit(dac.ProposalVerifier(e.State, e.Fx, e.Tx, tx)); err != nil {
+	if err := txProposal.VerifyWith(dac.ProposalVerifier(e.State, e.Fx, e.Tx, tx)); err != nil {
 		return fmt.Errorf("%w: %s", errInvalidProposal, err)
 	}
 
@@ -1995,27 +1995,14 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 
 	// verify ins and outs
 
-	bondTxIDsGetter := dac.NewProposalBondTxIDsGetter(e.State)
-
-	proposalIDs := append(tx.EarlyFinishedSuccessfulProposalIDs, tx.ExpiredSuccessfulProposalIDs...) //nolint:gocritic
-	additionalLockTxIDs := []ids.ID{}
-	for _, proposalID := range proposalIDs {
-		proposal, err := e.State.GetProposal(proposalID)
-		if err != nil {
-			return err
-		}
-		lockTxIDs, err := proposal.GetBondTxIDs(bondTxIDsGetter)
-		if err != nil {
-			return err
-		}
-		additionalLockTxIDs = append(additionalLockTxIDs, lockTxIDs...)
+	lockTxIDs, err := dac.GetBondTxIDs(e.State, tx)
+	if err != nil {
+		return err
 	}
 
-	proposalIDs = append(proposalIDs, tx.EarlyFinishedFailedProposalIDs...)
-	proposalIDs = append(proposalIDs, tx.ExpiredFailedProposalIDs...)
 	expectedIns, expectedOuts, err := e.FlowChecker.Unlock(
 		e.State,
-		append(proposalIDs, additionalLockTxIDs...),
+		lockTxIDs,
 		locked.StateBonded,
 	)
 	if err != nil {
@@ -2067,7 +2054,7 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 		}
 
 		// try to execute proposal
-		if err := proposal.Visit(dac.NewProposalExecutor(e.State, e.Fx)); err != nil {
+		if err := proposal.ExecuteWith(dac.NewProposalExecutor(e.State, e.Fx)); err != nil {
 			return err
 		}
 
@@ -2118,7 +2105,7 @@ func (e *CaminoStandardTxExecutor) FinishProposalsTx(tx *txs.FinishProposalsTx)
 		}
 
 		// try to execute proposal
-		if err := proposal.Visit(dac.NewProposalExecutor(e.State, e.Fx)); err != nil {
+		if err := proposal.ExecuteWith(dac.NewProposalExecutor(e.State, e.Fx)); err != nil {
 			return err
 		}
 

vms/platformvm/txs/executor/dac/camino_dac.go

@@ -17,8 +17,9 @@ import (
 )
 
 var (
-	_ dac.VerifierVisitor = (*proposalVerifier)(nil)
-	_ dac.ExecutorVisitor = (*proposalExecutor)(nil)
+	_ dac.Verifier        = (*proposalVerifier)(nil)
+	_ dac.Executor        = (*proposalExecutor)(nil)
+	_ dac.BondTxIDsGetter = (*proposalBondTxIDsGetter)(nil)
 
 	errNotConsortiumMember          = errors.New("address isn't consortium member")
 	errConsortiumMember             = errors.New("address is consortium member")
@@ -51,7 +52,7 @@ type proposalBondTxIDsGetter struct {
 	state state.Chain
 }
 
-func ProposalVerifier(state state.Chain, fx fx.Fx, signedTx *txs.Tx, tx *txs.AddProposalTx) dac.VerifierVisitor {
+func ProposalVerifier(state state.Chain, fx fx.Fx, signedTx *txs.Tx, tx *txs.AddProposalTx) dac.Verifier {
 	return &proposalVerifier{
 		state:               state,
 		fx:                  fx,
@@ -60,12 +61,30 @@ func ProposalVerifier(state state.Chain, fx fx.Fx, signedTx *txs.Tx, tx *txs.Add
 	}
 }
 
-func NewProposalExecutor(state state.Chain, fx fx.Fx) dac.ExecutorVisitor {
+func NewProposalExecutor(state state.Chain, fx fx.Fx) dac.Executor {
 	return &proposalExecutor{state: state, fx: fx}
 }
 
-func NewProposalBondTxIDsGetter(state state.Chain) dac.BondTxIDsGetter {
-	return &proposalBondTxIDsGetter{state: state}
+func GetBondTxIDs(state state.Chain, tx *txs.FinishProposalsTx) ([]ids.ID, error) {
+	return getBondTxIDs(&proposalBondTxIDsGetter{state: state}, state, tx)
+}
+
+// so we can test it with mock
+func getBondTxIDs(bondTxIDsGetter dac.BondTxIDsGetter, state state.Chain, tx *txs.FinishProposalsTx) ([]ids.ID, error) {
+	bondTxIDs := tx.ProposalIDs()
+	successfulProposalIDs := tx.SuccessfulProposalIDs()
+	for _, proposalID := range successfulProposalIDs {
+		proposal, err := state.GetProposal(proposalID)
+		if err != nil {
+			return nil, err
+		}
+		lockTxIDs, err := proposal.GetBondTxIDsWith(bondTxIDsGetter)
+		if err != nil {
+			return nil, err
+		}
+		bondTxIDs = append(bondTxIDs, lockTxIDs...)
+	}
+	return bondTxIDs, nil
 }
 
 // BaseFeeProposal