Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Feature) Allow cancellation of a ballot in VotingToManageEmissionFunds by its creator for 15 minutes after it's created #168

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

varasev
Copy link
Contributor

@varasev varasev commented Aug 15, 2018

@wkarshat
Copy link

wkarshat commented Sep 9, 2018

@varasev, perhaps this is the logic you are looking for in _finalize() of VotingToManageEmissionFunds.sol

    uint256 sendVotesCount = _getSendVotes(_id);
    uint256 burnVotesCount = _getBurnVotes(_id);
    uint256 freezeVotesCount = _getFreezeVotes(_id);
    QuorumStates quorumState = QuorumStates.Frozen;
        
    if (sendVotesCount > burnVotesCount)
    {
        if (sendVotesCount > freezeVotesCount)
            quorumState = QuorumStates.Sent;
    } else {
        if (burnVotesCount > sendVotesCount && burnVotesCount > freezeVotesCount)
            quorumState = QuorumStates.Burnt;
    }

However, is that desirable, could say Burn pass with 4:3:3 against Sent and Frozen?

@varasev
Copy link
Contributor Author

varasev commented Sep 10, 2018

@wkarshat Yes, that's a very good simplification, thank you! I merged your code in #181.

@wkarshat
Copy link

Sure.

What's the expected value, i.e. on Sokol, of:
uint256 validatorsLength = IPoaNetworkConsensus()

Can you help with understand how the section starting line 189 works?
releaseTime = releaseTime.add(releaseThreshold.mul(diff));
I would think you'd increment from CurrentTime and not releaseTime.

@varasev
Copy link
Contributor Author

varasev commented Sep 11, 2018

What's the expected value, i.e. on Sokol, of: uint256 validatorsLength =

There are 25 validators in Sokol for now (including MoC), so validatorsLength will be 24 (excluding MoC), because MoC has no voting rights.

Can you help with understand how the section starting line 189 works?

I'll try to explain it on the picture:

image

So, we have different timestamps called releaseTime on the timeline. They are separated by emissionReleaseThreshold which is 3 months (as described in poanetwork/RFC#14 (comment)).

Let's assume that the currentTime is 4 July and the current emissionReleaseTime in the smart contract is 1 January. E.g., we didn't create any ballot since 1 January, so emissionReleaseTime was not refreshed so far.

So, now is 4 July and we want to create a new ballot. We should refresh emissionReleaseTime and shift it to 1 July. When we call createBallot function, it will call refreshEmissionReleaseTime automatically.

According to

uint256 diff = currentTime.sub(releaseTime).div(releaseThreshold);

we have

diff = (4 July - 1 January) / 3 months = 2

Therefore

releaseTime = releaseTime.add(releaseThreshold.mul(diff));

will give us

releaseTime = 1 January + (3 months * 2)

i.e.

releaseTime = 1 January + 6 months = 1 July

@wkarshat
Copy link

So, let me see if I understand the mechanics of this vote, as I could not glean all of it from the code.

Starting on Oct 1st and for seven days thereafter, one of the Validators can introduce this type of a vote, and for 15 minutes after can cancel. Somehow, once this vote has been introduced (and not cancelled) no other Validator can introduce one of this type. Presumably, once cancelled it can be re-introduced. Presumably, once passed, this type of a vote cannot be re-introduced.

The minimum number of votes necessary to pass is 4 (quorum)
Voting has to complete within 7 days (after Oct 1st). BTW, what are the options for the duration, fixed or set by the Validator, within the limits of other vote types, like 48 hours?
The default behavior is Hold, in cases that the Burn or Payout did not win or quorum not achieved or time expired. BTW, I'm not sure I saw a test case for the last one.

In order to pass, Burn or Payout need most votes (more than either one of the other two options, no tie). So, in case of Burn:Pay:Hold votes of 7:6:5, Burn would pass.

@varasev
Copy link
Contributor Author

varasev commented Sep 13, 2018

Starting on Oct 1st and for seven days thereafter, one of the Validators can introduce this type of a vote, and for 15 minutes after can cancel. Somehow, once this vote has been introduced (and not cancelled) no other Validator can introduce one of this type. Presumably, once cancelled it can be re-introduced. Presumably, once passed, this type of a vote cannot be re-introduced.

That's right. Once passed, this type of a vote cannot be re-introduced until the next releaseTime timestamp (in 3 months after Oct 1st) and unless the previous ballot is finalized.

The minimum number of votes necessary to pass is 4 (quorum)

No, for the current set of validators in Sokol (24 persons excluding MoC) the minimum number will be 24 / 2 + 1 = 13.

That's defined here:

function _getGlobalMinThresholdOfVoters() internal view returns(uint256) {
return _getBallotsStorage().getProxyThreshold();
}

Voting has to complete within 7 days (after Oct 1st).

Yes, 7 days is the maximum period of time when validators can give their votes, so called distribution threshold, as described in poanetwork/RFC#14 (comment)

BTW, what are the options for the duration, fixed or set by the Validator, within the limits of other vote types, like 48 hours?

We have no minimum limit of duration of this type of a ballot assuming that the startTime and endTime timestamps will be calculated automatically and defined by Governance DApp when creating a new ballot. Also, validators might late and create a ballot, for example, on Oct 5th evening, when there are less than 48 hours left to the end of distribution threshold (Oct 7th). However, we could introduce such a minimum limit for this type of a ballot in VotingToManageEmissionFunds smart contract if necessary.

The default behavior is Hold, in cases that the Burn or Payout did not win or quorum not achieved or time expired. BTW, I'm not sure I saw a test case for the last one.

Yes, if time expired and quorum (or majority) not achieved, the finalize function will set frozen state:

_setQuorumState(_id, uint256(QuorumStates.Frozen));
and
_setQuorumState(_id, uint256(quorumState));

E.g., we have the next unit tests for those cases:

it('freeze funds if it did not pass minimum voters count', async () => {
await addValidator(votingKey2, miningKey2);
await addValidator(votingKey3, miningKey3);
await voting.createBallot(
VOTING_START_DATE, VOTING_END_DATE, receiver, "memo", {from: votingKey}
).should.be.fulfilled;
(await voting.getMinThresholdOfVoters.call(id)).should.be.bignumber.equal(2);
await voting.setTime(VOTING_START_DATE);
await voting.vote(id, choice.burn, {from: votingKey}).should.be.fulfilled;
await voting.setTime(VOTING_END_DATE + 1);
await voting.finalize(id, {from: votingKey3}).should.be.fulfilled;
(await voting.getQuorumState.call(id)).should.be.bignumber.equal(4);
(await web3.eth.getBalance(emissionFunds.address)).should.be.bignumber.equal(emissionFundsInitBalance);
emissionFundsInitBalance.should.be.bignumber.above(0);
});
it('freeze funds if there is no majority of 3 votes', async () => {
await voting.createBallot(
VOTING_START_DATE, VOTING_END_DATE, receiver, "memo", {from: votingKey}
).should.be.fulfilled;
await addValidator(votingKey2, miningKey2);
await addValidator(votingKey3, miningKey3);
await voting.setTime(VOTING_START_DATE);
await voting.vote(id, choice.send, {from: votingKey}).should.be.fulfilled;
await voting.vote(id, choice.burn, {from: votingKey2}).should.be.fulfilled;
await voting.vote(id, choice.freeze, {from: votingKey3}).should.be.fulfilled;
true.should.be.equal((await voting.getBallotInfo.call(id))[4]); // isFinalized
(await voting.getQuorumState.call(id)).should.be.bignumber.equal(4);
(await web3.eth.getBalance(emissionFunds.address)).should.be.bignumber.equal(emissionFundsInitBalance);
emissionFundsInitBalance.should.be.bignumber.above(0);
});
it('freeze funds if there is no majority of 4 votes', async () => {
await voting.createBallot(
VOTING_START_DATE, VOTING_END_DATE, receiver, "memo", {from: votingKey}
).should.be.fulfilled;
await addValidator(votingKey2, miningKey2);
await addValidator(votingKey3, miningKey3);
await addValidator(votingKey4, miningKey4);
await voting.setTime(VOTING_START_DATE);
await voting.vote(id, choice.send, {from: votingKey}).should.be.fulfilled;
await voting.vote(id, choice.burn, {from: votingKey2}).should.be.fulfilled;
await voting.vote(id, choice.send, {from: votingKey3}).should.be.fulfilled;
await voting.vote(id, choice.burn, {from: votingKey4}).should.be.fulfilled;
true.should.be.equal((await voting.getBallotInfo.call(id))[4]); // isFinalized
(await voting.getQuorumState.call(id)).should.be.bignumber.equal(4);
(await voting.getTotalVoters.call(id)).should.be.bignumber.equal(4);
(await web3.eth.getBalance(emissionFunds.address)).should.be.bignumber.equal(emissionFundsInitBalance);
emissionFundsInitBalance.should.be.bignumber.above(0);
});

In order to pass, Burn or Payout need most votes (more than either one of the other two options, no tie). So, in case of Burn:Pay:Hold votes of 7:6:5, Burn would pass.

Yes, right. A few cases for example:

Burn = 7 (pass, because greater than Pay and Hold)
Pay = 6
Hold = 5

Burn = 6
Pay = 6
Hold = 5 (pass, because of tie for Burn and Pay)

Burn = 6 (pass, because greater than Pay and Hold)
Pay = 5
Hold = 5

Burn = 5
Pay = 6 (pass, because greater than Burn and Hold)
Hold = 5

Burn = 4
Pay = 6 (pass, because greater than Burn and Hold)
Hold = 5

etc.

@wkarshat
Copy link

At which point does the check for 13 votes kick in, please?

@varasev
Copy link
Contributor Author

varasev commented Sep 14, 2018

It happens on the next line:

if (getTotalVoters(_id) < getMinThresholdOfVoters(_id)) {

When we call createBallot function, it calls VotingTo._createBallot:

uint256 ballotId = super._createBallot(

which sets MinThresholdOfVoters here:

_setMinThresholdOfVoters(ballotId, _getGlobalMinThresholdOfVoters());

It calls VotingToManageEmissionFunds._getGlobalMinThresholdOfVoters function which overrides VotingTo._getGlobalMinThresholdOfVoters.

The code of VotingToManageEmissionFunds._getGlobalMinThresholdOfVoters is:

function _getGlobalMinThresholdOfVoters() internal view returns(uint256) {
return _getBallotsStorage().getProxyThreshold();
}

I.e., it calls BallotsStorage.getProxyThreshold():

function getProxyThreshold() public view returns(uint256) {
return _getTotalNumberOfValidators().div(2).add(1);
}

@wkarshat
Copy link

@varasev, thanks hugely for the details. I'm impressed by how the system of smart contracts is layered and hangs together.
So, my understanding is that the smallest number of votes that can win is 5, if the votes are 5:4:4, for a total of 13.

@varasev
Copy link
Contributor Author

varasev commented Sep 17, 2018

@wkarshat

thanks hugely for the details. I'm impressed by how the system of smart contracts is layered and hangs together.

Thank you.

So, my understanding is that the smallest number of votes that can win is 5, if the votes are 5:4:4, for a total of 13.

Yes, right.

@shkron
Copy link

shkron commented Sep 19, 2018

@varasev, do I understand this correctly, in the scenario like this:

Burn = 12
Pay = 12
Hold = 0 (pass, because of tie for Burn and Pay)

Also could you please provide the definitions for those options.

UPD: also what happens in this case?

Burn = 0
Pay = 12
Hold = 12

@6proof
Copy link

6proof commented Sep 19, 2018

I think most Validators would expect minimum voting period to be 48 hours (as is case for other ballots). This would seem to be a minimum floor, particularly for subject of this type of ballot, and would hope this minimum would be added.

@varasev
Copy link
Contributor Author

varasev commented Sep 20, 2018

@shkron

do I understand this correctly, in the scenario like this:
Burn = 12
Pay = 12
Hold = 0 (pass, because of tie for Burn and Pay)

Yes, you do, because there is no majority of votes: Burn = Pay, so we can't make a concrete decision.

also what happens in this case?
Burn = 0
Pay = 12
Hold = 12

In this case Hold wins for the same reason as above. Hold always wins in case of no majority.

Also could you please provide the definitions for those options.

You can see conditions here:

if (getTotalVoters(_id) < getMinThresholdOfVoters(_id)) {
_setQuorumState(_id, uint256(QuorumStates.Frozen));
_emissionFunds.freezeFunds(amount);
return;
}
uint256 sendVotesCount = _getSendVotes(_id);
uint256 burnVotesCount = _getBurnVotes(_id);
uint256 freezeVotesCount = _getFreezeVotes(_id);
QuorumStates quorumState = QuorumStates.Frozen;
if (sendVotesCount > burnVotesCount) {
if (sendVotesCount > freezeVotesCount) {
quorumState = QuorumStates.Sent;
}
} else {
if (burnVotesCount > sendVotesCount && burnVotesCount > freezeVotesCount) {
quorumState = QuorumStates.Burnt;
}
}

@varasev
Copy link
Contributor Author

varasev commented Sep 20, 2018

@6proof ok, the corresponding issue has been opened: #190.

@vbaranov
Copy link
Collaborator

@varasev maybe, we could create a page in Governance section of wiki repo, that describes every threshold and the process overall? I assume it will be more transparent for everyone.

@varasev
Copy link
Contributor Author

varasev commented Sep 20, 2018

@vbaranov Sure, I'll create one.

@varasev
Copy link
Contributor Author

varasev commented Sep 27, 2018

maybe, we could create a page in Governance section of wiki repo, that describes every threshold and the process overall? I assume it will be more transparent for everyone.

Description of Emission Funds Ballot is now available on Wiki: https://github.com/poanetwork/wiki/wiki/Manage-Emission-Funds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants