From 4ac947856da1897a71b12a35529ef2ffd01ec49c Mon Sep 17 00:00:00 2001 From: sin3point14 Date: Fri, 29 Nov 2024 13:18:50 +0530 Subject: [PATCH] Correctly handle multiple passed upgrade proposals acc to cosmos-sdk If multiple passed upgrade proposals are in the "passed" state, the cosmos upgrade handler only treats the one with the highest proposal ID as "passed" and all other passed proposals as "cancelled". https://github.com/cosmos/cosmos-sdk/blob/f007a4ea0711da2bac20afc6283885c1b2496ae5/x/upgrade/keeper/keeper.go#L189-L193 --- internal/pkg/provider/chain/chain.go | 23 +++++++++++++++++++++ internal/pkg/provider/chain/chain_test.go | 25 +++++++++++++++++++++-- internal/pkg/provider/chain/types.go | 5 +++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/internal/pkg/provider/chain/chain.go b/internal/pkg/provider/chain/chain.go index 2b2197c..d57456e 100644 --- a/internal/pkg/provider/chain/chain.go +++ b/internal/pkg/provider/chain/chain.go @@ -86,6 +86,29 @@ func (p *Provider) GetUpgrades(ctx context.Context) ([]*urproto.Upgrade, error) } } + // If multiple passed upgrade proposals are in the "passed" state + // the cosmos upgrade handler only treats the one with the highest proposal ID + // as "passed" and all other passed proposals as "cancelled" + // this is not to be confused with the code above, which handles the + // case where multiple upgrade proposals exist for the same upgrade height + // https://github.com/cosmos/cosmos-sdk/blob/f007a4ea0711da2bac20afc6283885c1b2496ae5/x/upgrade/keeper/keeper.go#L189-L193 + latest_passed_proposal := uint64(0) + is_any_passed_proposal := false + for _, upgrade := range filtered { + if upgrade.Status == PASSED { + latest_passed_proposal = max(latest_passed_proposal, upgrade.ProposalID) + is_any_passed_proposal = true + } + } + + if is_any_passed_proposal { + for i := range filtered { + if filtered[i].Status == PASSED && filtered[i].ProposalID < latest_passed_proposal { + filtered[i].Status = CANCELLED + } + } + } + // sort upgrades in descending order by proposal id because iterating over map doesn't guarantee order sort.Slice(filtered, func(i, j int) bool { return filtered[i].ProposalID > filtered[j].ProposalID diff --git a/internal/pkg/provider/chain/chain_test.go b/internal/pkg/provider/chain/chain_test.go index 5b13ee0..8100883 100644 --- a/internal/pkg/provider/chain/chain_test.go +++ b/internal/pkg/provider/chain/chain_test.go @@ -61,18 +61,39 @@ func TestGetUpgrades(t *testing.T) { }, }, }, + { + name: "MultiplePassed", + proposals: v1.Proposals{ + newProposal(t, 1, 100, v1.StatusPassed), + newProposal(t, 2, 200, v1.StatusPassed), + }, + expected: []*urproto.Upgrade{ + { + Height: 200, + Type: urproto.UpgradeType_GOVERNANCE, + Status: urproto.UpgradeStatus_ACTIVE, + Source: urproto.ProviderType_CHAIN, + }, + { + Height: 100, + Type: urproto.UpgradeType_GOVERNANCE, + Status: urproto.UpgradeStatus_CANCELLED, + Source: urproto.ProviderType_CHAIN, + }, + }, + }, { name: "DuplicateProposalsWithPassedStatus", proposals: v1.Proposals{ newProposal(t, 1, 100, v1.StatusPassed), newProposal(t, 2, 100, v1.StatusPassed), - newProposal(t, 3, 200, v1.StatusPassed), + newProposal(t, 3, 200, v1.StatusDepositPeriod), }, expected: []*urproto.Upgrade{ { Height: 200, Type: urproto.UpgradeType_GOVERNANCE, - Status: urproto.UpgradeStatus_ACTIVE, + Status: urproto.UpgradeStatus_SCHEDULED, Source: urproto.ProviderType_CHAIN, }, { diff --git a/internal/pkg/provider/chain/types.go b/internal/pkg/provider/chain/types.go index 3989ed6..007fc47 100644 --- a/internal/pkg/provider/chain/types.go +++ b/internal/pkg/provider/chain/types.go @@ -21,6 +21,7 @@ const ( PASSED ProposalStatus = 3 REJECTED ProposalStatus = 4 FAILED ProposalStatus = 5 + CANCELLED ProposalStatus = 6 ) func (ps ProposalStatus) String() string { @@ -39,6 +40,8 @@ func (ps ProposalStatus) String() string { return "REJECTED" case FAILED: return "FAILED" + case CANCELLED: + return "CANCELLED" default: return fmt.Sprintf("%d", int(ps)) } @@ -70,6 +73,8 @@ func (cu chainUpgrade) ToProto() urproto.Upgrade { upgradeStatus = urproto.UpgradeStatus_CANCELLED case FAILED: upgradeStatus = urproto.UpgradeStatus_CANCELLED + case CANCELLED: + upgradeStatus = urproto.UpgradeStatus_CANCELLED } // #nosec G115