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) Votings to Manage Emission Funds #173

Merged
merged 10 commits into from
Sep 20, 2018
Merged

Conversation

varasev
Copy link
Contributor

@varasev varasev commented Sep 19, 2018

(Mandatory) Description

This PR solves the issue #130.

We have done 3rd HF in Sokol network, so now we have new smart contracts deployed there.

Thereby a new type of ballot has been added to Governance DApp with this PR. For now it is only supported for Sokol because we still have old contracts in Core.

Below are screenshots of DApp showing the new type of a ballot.

How the ballot's card looks:

image

Creator of a ballot can cancel the ballot within 15 minutes right after its creation (according to poanetwork/poa-network-consensus-contracts#161):

image

Here is how New Ballot page looks (it is not displayed for Core in this version):

image

I.e., a validator should enter a ballot description and paste an address of funds receiver. The field Current amount of funds is not editable and just shows the current balance of EmissionFunds contract.

Above the Add Ballot button, we see the note with time interval when the next ballot of this type can be created. The bounds of the interval are calculated automatically and depend on VotingToManageEmissionFunds contract's parameters such as release time, emission release threshold, and distribution threshold described here.

For Sokol the release time is equal to 20 Sep 2018 10:37 UTC, the emission release threshold is equal to 7 days, and distribution threshold is equal to 3 days.

Therefore, the nearest ballot of this type in Sokol can be started from 20 Sep 2018 10:37 UTC (the nearest release time) and it will be finished on 23 Sep 2018 10:37 UTC as shown on the screen above. I.e. the end date of the ballot will be 20 Sep 2018 10:37 UTC plus Distribution threshold.

The next ballot will be able to be started from 27 Sep 2018 10:37 UTC (next release time) and so on.

Some explanation of release time and emission release threshold can be found here: poanetwork/poa-network-consensus-contracts#168 (comment)

(Mandatory) What is it: (Fix), (Feature) or (Refactor)
(Feature)

@ghost ghost assigned varasev Sep 19, 2018
@ghost ghost added the in progress label Sep 19, 2018
@varasev varasev requested a review from vbaranov September 19, 2018 15:32
const { ballotStore, contractsStore } = this.props
const inputToMethod = {
startTime: startTime,
endTime: ballotStore.endTimeUnix,
endTime: endTime,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be written shorter:
startTime: startTime, => startTime,
endTime: endTime, => endTime,

const { ballotStore, contractsStore } = this.props
const inputToMethod = {
startTime: startTime,
endTime: ballotStore.endTimeUnix,
endTime: endTime,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same as above ☝️

const { ballotStore, contractsStore } = this.props
const inputToMethod = {
startTime: startTime,
endTime: ballotStore.endTimeUnix,
endTime: endTime,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same as above ☝️

createBallotForEmissionFunds = (startTime, endTime) => {
const { ballotStore, contractsStore } = this.props
const inputToMethod = {
startTime: startTime,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same as above ☝️

note = <p>To be able to create a new ballot, the previous ballot of this type must be finalized.</p>
}
if (contractsStore.netId === '77') {
explorerLink = `https://sokol-explorer.poa.network/account/${contractsStore.emissionFunds.address}`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest changing the link to Blockscout explorer:
https://blockscout.com/poa/sokol/address/${contractsStore.emissionFunds.address}

Also, I would suggest considering using eth-net-props npm package to get explorer URL for network by its ID (method getExplorerAccountLinkFor(addr, networkID)).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vbaranov Yes, but something's wrong with Blockscout displaying balance of EmissionFunds contract - it shows 0 POA for 0x523B6539Ff08d72A6C8Bb598Af95bF50c1EA839C but sokol-explorer shows correct balance: https://sokol-explorer.poa.network/account/0x523B6539Ff08d72A6C8Bb598Af95bF50c1EA839C

I think Blockscout doesn't take into account the balance that was accumulated not as a result of transactions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a bug. Was it reported in Blockscout repo?

Copy link
Contributor Author

@varasev varasev Sep 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've notified @acravenho about this and opened the issue blockscout/blockscout#767.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's use it until this issue will have not been fixed in Blockscout

onChange={e => ballotStore.changeBallotMetadata(e, 'receiver', 'ballotEmissionFunds')}
/>
<p className="hint">
The address to which the funds will be sent if `Send` operation obtains the majority of votes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, let's rewrite shorter: The address which the funds will be sent to, in case of the majority of votes ? What do you think @varasev?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

@observer
export class BallotEmissionFundsCard extends React.Component {
render() {
let { id, votingState, pos, contractsStore } = this.props
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters are not changed. Let's change the declaration to const.

.add(minBallotDurationInHours, 'hours')
.format()
let neededMinutes = moment(minEndTime).diff(moment(ballotStore.endTime), 'minutes')
let neededHours = Math.floor(neededMinutes / 60)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neededHours is not changed => const

.utc()
.add(14, 'days')
.format()
let exceededMinutes = moment(ballotStore.endTime).diff(moment(twoWeeks), 'minutes')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above: let to const

@ghost ghost added the in progress label Sep 20, 2018
@varasev varasev merged commit 5fdf932 into poanetwork:core Sep 20, 2018
@ghost ghost removed the in progress label Sep 20, 2018
@varasev varasev deleted the issue-130 branch September 20, 2018 10:29
@varasev
Copy link
Contributor Author

varasev commented Sep 27, 2018

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.

2 participants