Add checkVoteStatus to PFOffer.sol #217
Conversation
This is a port of @avsa's contribution from his proposed offer contract that can be seen here: https://gist.github.com/alexvandesande/122c05a0f8a7108523aed29ae7ec92b1 At any point `voteStatusDeadline` seconds before the end of the voting period anyone can call `checkVoteStatus()` to attempt and set the `wasApprovedBeforeDeadline` flag. This allows for a way to assure that the last minute yes vote does not occur by requiring the flag to be set to true at an earlier point of the voting deadline.
A test that the checkVoteStatus will fail if it is called after the `voteStatusDeadline` period
| @@ -98,6 +118,7 @@ contract PFOffer { | |||
| oneTimeCosts = _oneTimeCosts; | |||
| minDailyWithdrawLimit = _minDailyWithdrawLimit; | |||
| dailyWithdrawLimit = _minDailyWithdrawLimit; | |||
| proposalID = _proposalID; | |||
There was a problem hiding this comment.
The contractors doesn't know the proposal id at this point at it might take a while until it's whitelisted. They should have a separate functon watchContract or similar that will add it and check it:
https://gist.github.com/alexvandesande/122c05a0f8a7108523aed29ae7ec92b1#file-proposal-js-L271
There was a problem hiding this comment.
Yes I have considered that. It's easy to get the proposal ID. It's just always proposalsNumber()+1.
The problem with this approach is that you gotta make the proposal right after you deploy the contract. And even when you do you may collide with someone else who is making a proposal at the same time.
I had not thought of a separate watchContract function. That's a nice idea. But we need to have a way to prevent the contractor from setting it to a proposalID that's not theirs in order to fool the checkVoteStatus() function.
Probably the easiest way to do so is by means of community awareness. If the proposalID does not match the one that actually tries to sign() the contract we can put out a community alert that the contractor is a scammer.
There was a problem hiding this comment.
Check how I implemented the watch function to prevent that:
https://gist.github.com/alexvandesande/122c05a0f8a7108523aed29ae7ec92b1#file-proposal-js-L271
It checks if the address is the same and if the proposal has not been voted
yet. Someone could still trick it by submitting two proposals, but then the
token holders would be able to see that.
On Tue, Jun 7, 2016 at 9:02 AM, Lefteris Karapetsas <
notifications@github.com> wrote:
In PFOffer.sol
#217 (comment):@@ -98,6 +118,7 @@ contract PFOffer {
oneTimeCosts = _oneTimeCosts;
minDailyWithdrawLimit = _minDailyWithdrawLimit;
dailyWithdrawLimit = _minDailyWithdrawLimit;
proposalID = _proposalID;Yes I have considered that. It's easy to get the proposal ID. It's just
always proposalsNumber()+1.The problem with this approach is that you gotta make the proposal right
after you deploy the contract. And even when you do you may collide with
someone else who is making a proposal at the same time.I had not thought of a separate watchContract function. That's a nice
idea. But we need to have a way to prevent the contractor from setting it
to a proposalID that's not theirs in order to fool the checkVoteStatus()
function.Probably the easiest way to do so is by means of community awareness. If
the proposalID does not match the one that actually tries to sign() the
contract we can put out a community alert that the contractor is a scammer.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/slockit/DAO/pull/217/files/5620e943d777294e89ca7e1263df220847a10a92#r66057366,
or mute the thread
https://github.com/notifications/unsubscribe/AAG5AtCRRr7jH2XcVM5fm5YwwEVdi_LYks5qJV3rgaJpZM4IvUqK
.
Alex Van de Sande
Lead Designer
Ethereum Foundation
There was a problem hiding this comment.
Yes I had seen your implementation, but as you also point out it can be tricked by submitting two different proposals.
At the end I think that all these will be so easily detectable by the token holders that it does not matter at all. If such a thing happens the DAO observation team will put out an alert.
I am gonna port the function as is :)
Adding the watchProposal() function that @alexvandesande introduced in his Sample Offer contract [here](https://gist.github.com/alexvandesande/122c05a0f8a7108523aed29ae7ec92b1#file-proposal-js-L271). This way there is no requirement to pass the `proposalID` as a constructor argument. The contractor should instead add it on his own after he makes the proposal by calling `watchProposal()`. If the contractor tries to cheat by making 2 or more different proposals and adding the one that has gotten the most votes as the proposal ID in `watchProposal()` the token holders will notice and raise the alert so that the contractor can be blacklisted.
This PR is made to add the functionality of what @alexvandesande introduced in his sample Offer contract as
protectFromAmbush()to the Proposal Framework Offer contract located in this repository.The name of the function is changed to
checkVoteStatus(). At any point withinvoteStatusDeadlineseconds before the end of the voting period anyone can callcheckVoteStatus()to attempt and set thewasApprovedBeforeDeadlineflag.This allows for a way to protect against a last minute yes vote by requiring the flag to be set to true at an earlier point of the voting deadline. The offer can not be signed without this flag being set.
Tests for the functionality and logic introduced by
checkVoteStatus()are added and are passing.a note for the compilation of the contract: You need a very recent version of solc. It needs to contain this fix for the Dynamic Return Data Decoder bug.