From a7681da13e53e030fae801f2bfd2d47cd610274a Mon Sep 17 00:00:00 2001 From: dragosrebegea Date: Tue, 24 Sep 2024 15:28:33 +0300 Subject: [PATCH 1/5] MX-15031: Add cancel method before voting start --- vm/systemSmartContracts/governance.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/vm/systemSmartContracts/governance.go b/vm/systemSmartContracts/governance.go index 8501f73abf4..864eb538eda 100644 --- a/vm/systemSmartContracts/governance.go +++ b/vm/systemSmartContracts/governance.go @@ -684,7 +684,12 @@ func (g *governanceContract) closeProposal(args *vmcommon.ContractCallInput) vmc } currentEpoch := g.eei.BlockChainHook().CurrentEpoch() - if uint64(currentEpoch) <= generalProposal.EndVoteEpoch { + voteStarted := uint64(currentEpoch) >= generalProposal.StartVoteEpoch + if !g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) && !voteStarted { + voteStarted = true + } + voteEnded := uint64(currentEpoch) > generalProposal.EndVoteEpoch + if voteStarted && !voteEnded { g.eei.AddReturnMessage(fmt.Sprintf("proposal can be closed only after epoch %d", generalProposal.EndVoteEpoch)) return vmcommon.UserError } @@ -696,7 +701,7 @@ func (g *governanceContract) closeProposal(args *vmcommon.ContractCallInput) vmc return vmcommon.UserError } - generalProposal.Passed = g.computeEndResults(generalProposal, baseConfig) + generalProposal.Passed = g.computeEndResults(currentEpoch, generalProposal, baseConfig) if err != nil { g.eei.AddReturnMessage("computeEndResults error " + err.Error()) return vmcommon.UserError @@ -1018,7 +1023,13 @@ func (g *governanceContract) getTotalStakeInSystem() *big.Int { } // computeEndResults computes if a proposal has passed or not based on votes accumulated -func (g *governanceContract) computeEndResults(proposal *GeneralProposal, baseConfig *GovernanceConfigV2) bool { +func (g *governanceContract) computeEndResults(currentEpoch uint32, proposal *GeneralProposal, baseConfig *GovernanceConfigV2) bool { + voteStarted := uint64(currentEpoch) >= proposal.StartVoteEpoch + if !g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) && !voteStarted { + g.eei.Finish([]byte("Proposal closed before voting started")) + return true + } + totalVotes := big.NewInt(0).Add(proposal.Yes, proposal.No) totalVotes.Add(totalVotes, proposal.Veto) totalVotes.Add(totalVotes, proposal.Abstain) From f5da2c9777a9daae95dff8721e0e3a3c1468f039 Mon Sep 17 00:00:00 2001 From: dragosrebegea Date: Wed, 25 Sep 2024 12:42:48 +0300 Subject: [PATCH 2/5] MX-15031: refactor and tests --- vm/systemSmartContracts/governance.go | 26 +++++++----- vm/systemSmartContracts/governance_test.go | 49 +++++++++++++++++++--- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/vm/systemSmartContracts/governance.go b/vm/systemSmartContracts/governance.go index 864eb538eda..0124ccf37c9 100644 --- a/vm/systemSmartContracts/governance.go +++ b/vm/systemSmartContracts/governance.go @@ -683,13 +683,8 @@ func (g *governanceContract) closeProposal(args *vmcommon.ContractCallInput) vmc return vmcommon.UserError } - currentEpoch := g.eei.BlockChainHook().CurrentEpoch() - voteStarted := uint64(currentEpoch) >= generalProposal.StartVoteEpoch - if !g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) && !voteStarted { - voteStarted = true - } - voteEnded := uint64(currentEpoch) > generalProposal.EndVoteEpoch - if voteStarted && !voteEnded { + currentEpoch := uint64(g.eei.BlockChainHook().CurrentEpoch()) + if g.cannotClose(currentEpoch, generalProposal) { g.eei.AddReturnMessage(fmt.Sprintf("proposal can be closed only after epoch %d", generalProposal.EndVoteEpoch)) return vmcommon.UserError } @@ -736,6 +731,17 @@ func (g *governanceContract) closeProposal(args *vmcommon.ContractCallInput) vmc return vmcommon.Ok } +func (g *governanceContract) cannotClose(currentEpoch uint64, proposal *GeneralProposal) bool { + if !g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) { + return currentEpoch <= proposal.EndVoteEpoch + } + + voteStarted := currentEpoch >= proposal.StartVoteEpoch + voteEnded := currentEpoch > proposal.EndVoteEpoch + + return voteStarted && !voteEnded +} + func (g *governanceContract) clearEndedProposals(args *vmcommon.ContractCallInput) vmcommon.ReturnCode { if args.CallValue.Cmp(zero) != 0 { g.eei.AddReturnMessage("clearEndedProposals callValue expected to be 0") @@ -1023,9 +1029,9 @@ func (g *governanceContract) getTotalStakeInSystem() *big.Int { } // computeEndResults computes if a proposal has passed or not based on votes accumulated -func (g *governanceContract) computeEndResults(currentEpoch uint32, proposal *GeneralProposal, baseConfig *GovernanceConfigV2) bool { - voteStarted := uint64(currentEpoch) >= proposal.StartVoteEpoch - if !g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) && !voteStarted { +func (g *governanceContract) computeEndResults(currentEpoch uint64, proposal *GeneralProposal, baseConfig *GovernanceConfigV2) bool { + voteStarted := currentEpoch >= proposal.StartVoteEpoch + if g.enableEpochsHandler.IsFlagEnabled(common.GovernanceFixesFlag) && !voteStarted { g.eei.Finish([]byte("Proposal closed before voting started")) return true } diff --git a/vm/systemSmartContracts/governance_test.go b/vm/systemSmartContracts/governance_test.go index 168e3c7be60..c6fffdf00c9 100644 --- a/vm/systemSmartContracts/governance_test.go +++ b/vm/systemSmartContracts/governance_test.go @@ -1698,6 +1698,30 @@ func TestGovernanceContract_CloseProposalAfterGovernanceFixesShouldSetLastEndedN require.Equal(t, uint64(6), lastEndedNonce) } +func TestGovernanceContract_CannotClose(t *testing.T) { + t.Parallel() + + args := createMockGovernanceArgs() + gsc, _ := NewGovernanceContract(args) + + currentEpoch := uint64(10) + closedBeforeStart := &GeneralProposal{ + Yes: big.NewInt(20), + No: big.NewInt(0), + Veto: big.NewInt(0), + Abstain: big.NewInt(10), + StartVoteEpoch: currentEpoch + 1, + EndVoteEpoch: currentEpoch + 10, + } + + cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) + require.True(t, cannotClose) + + gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) + cannotClose = gsc.cannotClose(currentEpoch, closedBeforeStart) + require.False(t, cannotClose) +} + func TestGovernanceContract_ClearEndedProposalsCallValue(t *testing.T) { t.Parallel() @@ -2214,13 +2238,28 @@ func TestComputeEndResults(t *testing.T) { } gsc, _ := NewGovernanceContract(args) + startVoteEpoch := uint64(10) + gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) + closedBeforeStart := &GeneralProposal{ + Yes: big.NewInt(20), + No: big.NewInt(0), + Veto: big.NewInt(0), + Abstain: big.NewInt(10), + StartVoteEpoch: startVoteEpoch, + } + passed := gsc.computeEndResults(startVoteEpoch-1, closedBeforeStart, baseConfig) + require.True(t, passed) + require.Equal(t, "Proposal closed before voting started", retMessage) + require.False(t, closedBeforeStart.Passed) + + gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub() didNotPassQuorum := &GeneralProposal{ Yes: big.NewInt(20), No: big.NewInt(0), Veto: big.NewInt(0), Abstain: big.NewInt(10), } - passed := gsc.computeEndResults(didNotPassQuorum, baseConfig) + passed = gsc.computeEndResults(startVoteEpoch, didNotPassQuorum, baseConfig) require.False(t, passed) require.Equal(t, "Proposal did not reach minQuorum", retMessage) require.False(t, didNotPassQuorum.Passed) @@ -2231,7 +2270,7 @@ func TestComputeEndResults(t *testing.T) { Veto: big.NewInt(0), Abstain: big.NewInt(10), } - passed = gsc.computeEndResults(didNotPassVotes, baseConfig) + passed = gsc.computeEndResults(startVoteEpoch, didNotPassVotes, baseConfig) require.False(t, passed) require.Equal(t, "Proposal rejected", retMessage) require.False(t, didNotPassVotes.Passed) @@ -2242,7 +2281,7 @@ func TestComputeEndResults(t *testing.T) { Veto: big.NewInt(0), Abstain: big.NewInt(10), } - passed = gsc.computeEndResults(didNotPassVotes2, baseConfig) + passed = gsc.computeEndResults(startVoteEpoch, didNotPassVotes2, baseConfig) require.False(t, passed) require.Equal(t, "Proposal rejected", retMessage) require.False(t, didNotPassVotes2.Passed) @@ -2253,7 +2292,7 @@ func TestComputeEndResults(t *testing.T) { Veto: big.NewInt(70), Abstain: big.NewInt(10), } - passed = gsc.computeEndResults(didNotPassVeto, baseConfig) + passed = gsc.computeEndResults(startVoteEpoch, didNotPassVeto, baseConfig) require.False(t, passed) require.Equal(t, "Proposal vetoed", retMessage) require.False(t, didNotPassVeto.Passed) @@ -2264,7 +2303,7 @@ func TestComputeEndResults(t *testing.T) { Veto: big.NewInt(10), Abstain: big.NewInt(10), } - passed = gsc.computeEndResults(pass, baseConfig) + passed = gsc.computeEndResults(startVoteEpoch, pass, baseConfig) require.True(t, passed) require.Equal(t, "Proposal passed", retMessage) } From 1a2a5055b4fbd6ed5a108848dda2b744f950fcce Mon Sep 17 00:00:00 2001 From: dragosrebegea Date: Wed, 25 Sep 2024 12:53:51 +0300 Subject: [PATCH 3/5] MX-15031: add extra check --- vm/systemSmartContracts/governance_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/vm/systemSmartContracts/governance_test.go b/vm/systemSmartContracts/governance_test.go index c6fffdf00c9..317e6d84af6 100644 --- a/vm/systemSmartContracts/governance_test.go +++ b/vm/systemSmartContracts/governance_test.go @@ -1705,13 +1705,15 @@ func TestGovernanceContract_CannotClose(t *testing.T) { gsc, _ := NewGovernanceContract(args) currentEpoch := uint64(10) + startVoteEpoch := currentEpoch + 1 + endVoteEpoch := currentEpoch + 10 closedBeforeStart := &GeneralProposal{ Yes: big.NewInt(20), No: big.NewInt(0), Veto: big.NewInt(0), Abstain: big.NewInt(10), - StartVoteEpoch: currentEpoch + 1, - EndVoteEpoch: currentEpoch + 10, + StartVoteEpoch: startVoteEpoch, + EndVoteEpoch: endVoteEpoch, } cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) @@ -1720,6 +1722,9 @@ func TestGovernanceContract_CannotClose(t *testing.T) { gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) cannotClose = gsc.cannotClose(currentEpoch, closedBeforeStart) require.False(t, cannotClose) + + cannotClose = gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) + require.True(t, cannotClose) } func TestGovernanceContract_ClearEndedProposalsCallValue(t *testing.T) { From 499b2f6a1fabc9201575c4afd7c1f9d37e2a367d Mon Sep 17 00:00:00 2001 From: dragosrebegea Date: Wed, 25 Sep 2024 12:55:34 +0300 Subject: [PATCH 4/5] MX-15031: fix test --- vm/systemSmartContracts/governance_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vm/systemSmartContracts/governance_test.go b/vm/systemSmartContracts/governance_test.go index 317e6d84af6..fbea5c9d83b 100644 --- a/vm/systemSmartContracts/governance_test.go +++ b/vm/systemSmartContracts/governance_test.go @@ -1719,12 +1719,12 @@ func TestGovernanceContract_CannotClose(t *testing.T) { cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) require.True(t, cannotClose) + cannotClose = gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) + require.False(t, cannotClose) + gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) cannotClose = gsc.cannotClose(currentEpoch, closedBeforeStart) require.False(t, cannotClose) - - cannotClose = gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) - require.True(t, cannotClose) } func TestGovernanceContract_ClearEndedProposalsCallValue(t *testing.T) { From 7791a434cd81767a76fa57b20b618a39fd366afb Mon Sep 17 00:00:00 2001 From: dragosrebegea Date: Wed, 25 Sep 2024 13:39:32 +0300 Subject: [PATCH 5/5] MX-15031: add boundary tests --- vm/systemSmartContracts/governance_test.go | 71 +++++++++++++++++++--- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/vm/systemSmartContracts/governance_test.go b/vm/systemSmartContracts/governance_test.go index fbea5c9d83b..b59c8b5935f 100644 --- a/vm/systemSmartContracts/governance_test.go +++ b/vm/systemSmartContracts/governance_test.go @@ -1716,15 +1716,72 @@ func TestGovernanceContract_CannotClose(t *testing.T) { EndVoteEpoch: endVoteEpoch, } - cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) - require.True(t, cannotClose) + t.Run("fixes flag disabled, should return false only after end vote epoch", func(t *testing.T) { + t.Run("current epoch is less than start epoch", func(t *testing.T) { + t.Parallel() - cannotClose = gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) - require.False(t, cannotClose) + cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is equal with start epoch", func(t *testing.T) { + t.Parallel() - gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) - cannotClose = gsc.cannotClose(currentEpoch, closedBeforeStart) - require.False(t, cannotClose) + cannotClose := gsc.cannotClose(startVoteEpoch, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is between start and end epochs", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(currentEpoch+5, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is equal with end epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(endVoteEpoch, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is larger than end epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) + require.False(t, cannotClose) + }) + }) + t.Run("fixes flag enabled, should return false only between start and end epoch", func(t *testing.T) { + gsc.enableEpochsHandler = enableEpochsHandlerMock.NewEnableEpochsHandlerStub(common.GovernanceFixesFlag) + + t.Run("current epoch is less than start epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(currentEpoch, closedBeforeStart) + require.False(t, cannotClose) + }) + t.Run("current epoch is equal with start epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(startVoteEpoch, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is between start and end epochs", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(currentEpoch+5, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is equal with end epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(endVoteEpoch, closedBeforeStart) + require.True(t, cannotClose) + }) + t.Run("current epoch is larger than end epoch", func(t *testing.T) { + t.Parallel() + + cannotClose := gsc.cannotClose(endVoteEpoch+1, closedBeforeStart) + require.False(t, cannotClose) + }) + }) } func TestGovernanceContract_ClearEndedProposalsCallValue(t *testing.T) {