vote and effective after upgrade proposal (#3120)

* vote and effective after upgrade proposal * remove waiting queue * fix test --------- Co-authored-by: yangzhe <[email protected]>
okx · Apr 27, 2023 · 761d968 · 761d968
1 parent 95eb909
commit 761d968
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 30 deletions.
diff --git a/x/params/proposal_handler_test.go b/x/params/proposal_handler_test.go
@@ -246,8 +246,8 @@ func (suite *ProposalHandlerSuite) TestCheckUpgradeVote() {
 	}{
 		{0, 10, false},
 		{0, 1111, false},
-		{10, 11, true},
-		{10, 10, true},
+		{10, 11, false},
+		{10, 10, false},
 		{10, 9, false},
 	}
 

diff --git a/x/params/upgrade_executor.go b/x/params/upgrade_executor.go
@@ -5,7 +5,6 @@ import (
 	"math"
 
 	sdk "github.com/okex/exchain/libs/cosmos-sdk/types"
-	sdkerrors "github.com/okex/exchain/libs/cosmos-sdk/types/errors"
 	"github.com/okex/exchain/x/common"
 	govtypes "github.com/okex/exchain/x/gov/types"
 	"github.com/okex/exchain/x/params/types"
@@ -35,6 +34,7 @@ func handleUpgradeProposal(ctx sdk.Context, k *Keeper, proposalID uint64, propos
 		_ = storeWaitingUpgrade(ctx, k, proposal, effectiveHeight) // ignore error
 		return nil
 	}
+	defer k.gk.RemoveFromWaitingProposalQueue(ctx, confirmHeight, proposalID)
 
 	// proposal will be confirmed right now, check if ready.
 	cbs, ready := k.queryReadyForUpgrade(proposal.Name)
@@ -77,11 +77,8 @@ func getUpgradeProposalConfirmHeight(currentHeight uint64, proposal types.Upgrad
 
 	if confirmHeight < currentHeight {
 		// if it's too late to make the proposal become effective at the height which we expected,
-		// refuse to effective this proposal
-		return 0, sdkerrors.New(DefaultCodespace, types.BaseParamsError,
-			fmt.Sprintf("current height '%d' has exceed "+
-				"the expect height '%d' of upgrade proposal '%s'",
-				currentHeight, proposal.ExpectHeight, proposal.Name))
+		// make the upgrade effective at next block (just like height is not specified).
+		confirmHeight = currentHeight
 	}
 	return confirmHeight, nil
 }
@@ -146,15 +143,6 @@ func checkUpgradeValidEffectiveHeight(ctx sdk.Context, k *Keeper, effectiveHeigh
 	return nil
 }
 
-func checkUpgradeVote(ctx sdk.Context, proposalID uint64, proposal types.UpgradeProposal, _ govtypes.Vote) (string, sdk.Error) {
-	curHeight := uint64(ctx.BlockHeight())
-
-	if proposal.ExpectHeight != 0 && proposal.ExpectHeight <= curHeight {
-		return "", sdkerrors.New(DefaultCodespace, types.BaseParamsError,
-			fmt.Sprintf("can not vote： current height '%d' has exceed "+
-				"the expect height '%d' of upgrade proposal '%s'(proposal id '%d')",
-				curHeight, proposal.ExpectHeight, proposal.Name, proposalID))
-	}
-
+func checkUpgradeVote(_ sdk.Context, _ uint64, _ types.UpgradeProposal, _ govtypes.Vote) (string, sdk.Error) {
 	return "", nil
 }
diff --git a/x/params/upgrade_executor_test.go b/x/params/upgrade_executor_test.go
@@ -97,8 +97,8 @@ func TestUpgradeProposalConfirmHeight(t *testing.T) {
 		expectConfirmHeight  uint64
 	}{
 		{uint64(10), uint64(0), false, uint64(10)},
-		{uint64(10), uint64(5), true, uint64(0)},
-		{uint64(10), uint64(10), true, uint64(0)},
+		{uint64(10), uint64(5), false, uint64(10)},
+		{uint64(10), uint64(10), false, uint64(10)},
 		{uint64(10), uint64(11), false, uint64(10)},
 		{uint64(10), uint64(15), false, uint64(14)},
 	}
@@ -259,8 +259,8 @@ func (suite *UpgradeInfoStoreSuite) TestCheckUpgradeVote() {
 	}{
 		{0, 10, false},
 		{0, 1111, false},
-		{10, 11, true},
-		{10, 10, true},
+		{10, 11, false},
+		{10, 10, false},
 		{10, 9, false},
 	}
 
@@ -287,19 +287,25 @@ func (suite *UpgradeInfoStoreSuite) TestHandleUpgradeProposal() {
 		expectHitError        bool
 	}{
 		{ // expect height is not zero but less than current height
-			expectHeight: 10, currentHeight: 10, claimReady: false, expectPanic: false, expect1stExecuteError: true,
+			expectHeight: 10, currentHeight: 10, claimReady: false, expectPanic: true, expect1stExecuteError: false,
 		},
 		{ // expect height is not zero but only greater than current height 1; and not claim ready
-			expectHeight: 11, currentHeight: 10, claimReady: false, expectPanic: true,
+			expectHeight: 21, currentHeight: 20, claimReady: false, expectPanic: true,
 		},
 		{ // expect height is not zero and greater than current height; but not claim ready
-			expectHeight: 12, currentHeight: 10, claimReady: false, expectPanic: true, expect1stExecuteError: false,
+			expectHeight: 32, currentHeight: 30, claimReady: false, expectPanic: true, expect1stExecuteError: false,
 		},
 		{ // everything's ok: expect height is not zero and greater than current height; and claim ready
-			expectHeight: 12, currentHeight: 10, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
+			expectHeight: 42, currentHeight: 40, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
 		},
 		{ // everything's ok: expect height is zero and claim ready
-			expectHeight: 0, currentHeight: 10, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
+			expectHeight: 0, currentHeight: 50, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
+		},
+		{ // everything's ok: expect height is not zero and equal to current height; and claim ready
+			expectHeight: 60, currentHeight: 60, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
+		},
+		{ // everything's ok: expect height is not zero and less than current height; and claim ready
+			expectHeight: 70, currentHeight: 72, claimReady: true, expectPanic: false, expect1stExecuteError: false, expectHitError: false,
 		},
 	}
 
@@ -316,6 +322,9 @@ func (suite *UpgradeInfoStoreSuite) TestHandleUpgradeProposal() {
 		if tt.expectHeight == 0 {
 			confirmHeight = tt.currentHeight
 		}
+		if confirmHeight < tt.currentHeight {
+			confirmHeight = tt.currentHeight
+		}
 		effectiveHeight := confirmHeight + 1
 
 		cbCount := 0
@@ -327,7 +336,7 @@ func (suite *UpgradeInfoStoreSuite) TestHandleUpgradeProposal() {
 			})
 		}
 
-		if tt.expectPanic && confirmHeight == tt.currentHeight {
+		if tt.expectPanic && confirmHeight <= tt.currentHeight {
 			suite.Panics(func() { _ = handler(ctx, proposal) })
 			continue
 		}
@@ -339,8 +348,7 @@ func (suite *UpgradeInfoStoreSuite) TestHandleUpgradeProposal() {
 			continue
 		}
 
-		suite.GreaterOrEqual(confirmHeight, tt.currentHeight)
-		if confirmHeight != tt.currentHeight {
+		if confirmHeight > tt.currentHeight {
 			// proposal is inserted to gov waiting queue, execute it
 			expectInfo := types.UpgradeInfo{
 				Name:            upgradeProposal.Name,