skmgoldin / tcr Goto Github PK
View Code? Open in Web Editor NEWA generic token-curated registry
License: Apache License 2.0
A generic token-curated registry
License: Apache License 2.0
Steps to reproduce:
Here is a test case demonstrating this issue #2
The fix is here skmgoldin/sol-dll#1
now + get("pApplyStageLen")
and now + get("pApplyStageLen") + get("pCommitStageLen") + get("pRevealStageLen") + PROCESSBY
can overflow.
Recommendation
Use SafeMath for arithmetic, and assert that these calculations succeed in processProposal
so proposals can't get the contract in a state where the arithmetic for new proposals will revert every transaction.
what happens when _name
is not one of the reparameterizable parameters?
function proposeReparameterization(string _name, uint _value) public returns (bytes32) {
uint deposit = get("pMinDeposit");
bytes32 propID = keccak256(_name, _value);
require(!propExists(propID)); // Forbid duplicate proposals
require(get(_name) != _value); // Forbid NOOP reparameterizations
require(token.transferFrom(msg.sender, this, deposit)); // escrow tokens (deposit amt)
// attach name and value to pollID
proposals[propID] = ParamProposal({
appExpiry: now + get("pApplyStageLen"),
challengeID: 0,
deposit: deposit,
name: _name,
owner: msg.sender,
processBy: now + get("pApplyStageLen") + get("pCommitStageLen") +
get("pRevealStageLen") + PROCESSBY,
value: _value
});
_ReparameterizationProposal(msg.sender, _name, _value, propID);
return propID;
}
function processProposal(bytes32 _propID) public {
ParamProposal storage prop = proposals[_propID];
if (canBeSet(_propID)) {
set(prop.name, prop.value);
} else if (challengeCanBeResolved(_propID)) {
resolveChallenge(_propID);
} else if (now > prop.processBy) {
require(token.transfer(prop.owner, prop.deposit));
} else {
revert();
}
delete proposals[_propID];
}
Running node 8.11.1, trying to deploy to any test network and receiving the following error.
Running migration: 3_optional_plcr_factory.js
Linking DLL to PLCRFactory
Linking AttributeStore to PLCRFactory
Deploying PLCRFactory...
... 0x69052b83a484f82fa7444b580ac23061dc9d0caa29cfce34e35bb9abb0886eda
Error encountered, bailing. Network state unknown. Review successful transactions manually.
Error: The contract code couldn't be stored, please check your gas amount.
at Object.callback (/Users/drewstone/src/eth/cw/tcr/node_modules/truffle/build/webpack:/~/web3/lib/web3/contract.js:147:1)
at /Users/drewstone/src/eth/cw/tcr/node_modules/truffle/build/webpack:/~/web3/lib/web3/method.js:142:1
at /Users/drewstone/src/eth/cw/tcr/node_modules/truffle/build/webpack:/~/web3/lib/web3/requestmanager.js:89:1
at /Users/drewstone/src/eth/cw/tcr/node_modules/truffle/build/webpack:/~/truffle-migrate/index.js:225:1
at /Users/drewstone/src/eth/cw/tcr/node_modules/truffle/build/webpack:/~/truffle-provider/wrapper.js:134:1
at /Users/drewstone/src/eth/cw/tcr/node_modules/web3-provider-engine/index.js:152:9
at /Users/drewstone/src/eth/cw/tcr/node_modules/async/internal/once.js:12:16
at replenish (/Users/drewstone/src/eth/cw/tcr/node_modules/async/internal/eachOfLimit.js:61:25)
at /Users/drewstone/src/eth/cw/tcr/node_modules/async/internal/eachOfLimit.js:71:9
at eachLimit (/Users/drewstone/src/eth/cw/tcr/node_modules/async/eachLimit.js:43:36)
at /Users/drewstone/src/eth/cw/tcr/node_modules/async/internal/doLimit.js:9:16
at end (/Users/drewstone/src/eth/cw/tcr/node_modules/web3-provider-engine/index.js:127:5)
at /Users/drewstone/src/eth/cw/tcr/node_modules/web3-provider-engine/subproviders/provider.js:20:5
at XMLHttpRequest.request.onreadystatechange (/Users/drewstone/src/eth/cw/tcr/node_modules/web3/lib/web3/httpprovider.js:118:13)
at XMLHttpRequestEventTarget.dispatchEvent (/Users/drewstone/src/eth/cw/tcr/node_modules/web3/node_modules/xhr2/lib/xhr2.js:64:18)
at XMLHttpRequest._setReadyState (/Users/drewstone/src/eth/cw/tcr/node_modules/web3/node_modules/xhr2/lib/xhr2.js:354:12)
at XMLHttpRequest._onHttpResponseEnd (/Users/drewstone/src/eth/cw/tcr/node_modules/web3/node_modules/xhr2/lib/xhr2.js:509:12)
at IncomingMessage.<anonymous> (/Users/drewstone/src/eth/cw/tcr/node_modules/web3/node_modules/xhr2/lib/xhr2.js:469:24)
at emitNone (events.js:111:20)
at IncomingMessage.emit (events.js:208:7)
at endReadableNT (_stream_readable.js:1064:12)
at _combinedTickCallback (internal/process/next_tick.js:138:11)
at process._tickCallback (internal/process/next_tick.js:180:9)
Hey all,
While parsing and handling TCR data I came across a bit of a need to have some standards or "suggested standards" around TCR data. Below I want to walk through some potential examples, and ideas around standard TCR data formatting.
Many apps and systems will need some kind of standard API or data format to fallback on for some data primitives. It would be nice to have some basic suggested standards around this to simplify third-party system architecture.
The plaintext option is simply the data in question and does not need to be formatted.
data => "nick.com"
data => "MyNameIsThis"
data => "Hello world"
This option describes an IPFS hash, which should lead to a reducible JSON data schema
data => QmTp2hEo8eXRp6wg7jXv1BLCMh5a4F3B7buAUZNZUu772j
JSON Data Format:
QmTp2hEo8eXRp6wg7jXv1BLCMh5a4F3B7buAUZNZUu772j =>
{
"title": "My application title",
"name": "Data Name",
"description": "My application description",
"shortDescription": "My short application description",
"content": "My application content"
}
JSON Schemas can follow some existing standards if possible:
Products: http://schema.org/Product
Persons: http://schema.org/Person
Offers: http://schema.org/Offer
Buisineses: http://schema.org/LocalBusiness
Thoughts welcome!
determineReward
: gets the number of tokens for the winning party of a challenge (Applicant or Challenger, not Voter)How about challengeWinnerReward
? (same implementation as the now deprecated #10)
claimReward
: only deals with Voter.How about claimVoterReward
? (same implementation as the now deprecated #10)
tokenClaims
: sounds like it would return true
if a voter has claim to some tokens. however the opposite is true. (clearly i was confused in #9)How about voterClaimedReward
?
Hi,
I'm using:
node v8.11.3
npm v5.6.0
web3 v1.0.0-beta.35
and I can't install the tcr. Here's what happens
$ npm install
[email protected] install /home/sebi/bc/tcr
truffle install
Error: Invalid JSON RPC response: "" at Object.InvalidResponse (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/web3/lib/web3/errors.js:38:1) at XMLHttpRequest.request.onreadystatechange (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/web3/lib/web3/httpprovider.js:125:1) at XMLHttpRequestEventTarget.dispatchEvent (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/xhr2/lib/xhr2.js:64:1) at XMLHttpRequest._setReadyState (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/xhr2/lib/xhr2.js:354:1) at XMLHttpRequest._onHttpRequestError (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/xhr2/lib/xhr2.js:544:1) at ClientRequest.<anonymous> (/home/sebi/bc/tcr/node_modules/truffle/build/webpack:/~/xhr2/lib/xhr2.js:414:1) at emitOne (events.js:116:13) at ClientRequest.emit (events.js:211:7) at TLSSocket.socketErrorListener (_http_client.js:387:9) at emitOne (events.js:116:13)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install:
truffle install
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR! /home/sebi/.npm/_logs/2018-08-13T19_03_53_085Z-debug.log
Any ideas on what the problem might be?
Thanks!
Does anyone object to Apache-2?
I find it unclear from the name what updateStatus()
is supposed to do, which makes it difficult to remember when it is necessary to call it.
From the code, it looks like it can either
Is there some name that covers both aspects? Otherwise, it might make sense to split updateStatus()
into two separate functions. Something like finalizeApplication()
and finalizeChallenge()
?
As we know, in Partial Lock Commit Reveal voting (PLCR), voters can vote with the same tokens across multiple polls. This surfaces a few issues when a TCR has MINORITY_BLOC_SLASH (MBS) implemented in order to deter vote splitting. Here’s why:
If MINORITY_BLOC_SLASH is set to 100% and a voter votes across multiple polls with the same tokens within a PLCR environment, and once the voter ends up in a losing bloc (100% of their tokens are taken due to MINORITY_BLOC_SLASH being set at 100%), there won’t be any token left for that voter to cover the debt for any remaining polls still open.
We can solve for this by setting the MINORITY_BLOC_SLASH lower than 100%. Let’s say that the Minority_Bloc_Slash is set to 10%.
At a 10% Minority_Bloc_Slash, we still run into issues.
For example, if a voter uses 100 tokens to vote across 11 polls and the Min_Bloc_Slash is set to 10%. If the voter loses in every poll then 10% is taken out of 100 tokens 11 times, we can see the dilemma. In this scenario, the user will end up with a negative token balance: 100 - 110 = -10.
We can solve for this however by implementing the MINORITY_BLOC_SLASH to deduct tokens in chronological order. For example, taking the above scenario of a voter using 100 tokens to vote across multiple polls with a MINORITY_BLOC_SLASH set to 10%. See below.
For TCR’s using PLCR in conjunction with MINORITY_BLOC_SLASH (as a deterrent for vote splitting), the MINORITY_BLOC_SLASH shouldn’t be set to 100%.
If MINORITY_BLOC_SLASH is set to a percentage less than 100%, then in order to prevent a voter who is voting with the same tokens across multiple polls from ending with a negative balance, we should deduct the MINORITY_BLOC_SLASH percentage chronologically (or implement some other logic to address the potential for a voter to end with a negative balance).
If PLCRVoting, Parameterizer, and Registry work with any EIP20 token, use the EIP20Interface type rather than ConsenSys's specific implementation of it.
Throughout the contract system, calls are made to the token contract followed by state changes in the contracts themselves. Token contracts can perform reentrancy attacks to take advantage of this.
Recommendation
Move token contract calls after all state changes in each function. Alternatively, document that TCRs must use trusted token contracts that cannot call other contracts.
Hello,
I was wondering what's the use of masterCopy
var that's used in all 3 contracts ? i'm guessing it's for creating copies of contracts safely as mentioned here ethereum/solidity#2296
if so, my noob questions are:
masterCopy
set ? I looked at setup
and the constructor but can't seem to find it. I also looked at the tests but couldn't find that out.I appreciate you taking the time to answer these questions. and I do apologize if they seem like simple questions 🙇 😄
Cheers.
listingHash.applicationExpiry = block.timestamp + parameterizer.get("applyStageLen")
can overflow.
Recommendation
Use SafeMath for arithmetic. If the token holders set applyStageLen
too high, all new applications will revert until they pass a proposal to change the setting.
In addition to a version
, a TCR should have a name
like "AdChain Registry".
If pMinDeposit
is changed after a proposal has been created, tokens will not be distributed properly.
If pMinDeposit
is increased, challenge.stake
and challenge.rewardPool
will be based on a larger value than what the proposer deposited. Winning challengers will attempt to withdraw (2 * challenges[_challengeID].stake) - challenges[_challengeID].rewardPool
, which will fail if the deposit has been increased enough to make the reward pool larger than the proposer's deposit. Even when it succeeds, the reward pool will be larger than the balance that remains the contract, so some voters won't get rewarded.
If pMinDeposit
is decreased, challenge.stake
and challenge.rewardPool
will be based on a smaller value than what the proposer deposited. Winning proposed will withdraw (2 * challenges[_challengeID].stake) - challenges[_challengeID].rewardPool
, which will leave some tokens allocated neither to the winning proposer nor the winning voters. The tokens will remain stuck in the contract.
Recommendation
In challengeReparameterization
, don't reference pMinDeposit
. Reference proposal.deposit
so challenges are always consistent with their proposals.
Whitelisted applicants can exit the registry when they discover new information that threatens their deposit. New applicants cannot do so because exit
calls require(isWhitelisted(_listingHash))
. Even without an active challenge, a new applicant with new information cannot protect themselves from a challenge the way whitelisted applicants can.
Recommendation
If this is desired behavior, keep it. Just pointing out the discrepancy.
_Application
: add appEndDate
_Challenge
: remove deposit
, add challenger
, change pollID
to challengeID
_NewListingWhitelisted
: change to _ApplicationWhitelisted
_ListingWithdrawn(bytes32 listingHash);
_TouchAndRemoved(bytes32 listingHash);
Current Implementation:
event _Application(bytes32 listingHash, uint deposit, string data);
event _Challenge(bytes32 listingHash, uint deposit, uint pollID, string data);
event _Deposit(bytes32 listingHash, uint added, uint newTotal);
event _Withdrawal(bytes32 listingHash, uint withdrew, uint newTotal);
event _NewListingWhitelisted(bytes32 listingHash);
event _ApplicationRemoved(bytes32 listingHash);
event _ListingRemoved(bytes32 listingHash);
event _ChallengeFailed(uint challengeID);
event _ChallengeSucceeded(uint challengeID);
event _RewardClaimed(address voter, uint challengeID, uint reward);
Proposed implementation:
* event _Application(bytes32 listingHash, uint deposit, uint appEndDate, string data);
* event _Challenge(bytes32 listingHash, address challenger, uint challengeID, string data);
event _Deposit(bytes32 listingHash, uint added, uint newTotal);
event _Withdrawal(bytes32 listingHash, uint withdrew, uint newTotal);
* event _ApplicationWhitelisted(bytes32 listingHash);
event _ApplicationRemoved(bytes32 listingHash);
event _ListingRemoved(bytes32 listingHash);
+ event _ListingWithdrawn(bytes32 listingHash);
+ event _TouchAndRemoved(bytes32 listingHash);
event _ChallengeFailed(uint challengeID);
event _ChallengeSucceeded(uint challengeID);
event _RewardClaimed(address voter, uint challengeID, uint reward);
Add: (#34)
_RewardClaimed(msg.sender, _challengeID, reward);
Add: (#33)
_ProposalAccepted(bytes32 propID);
_ProposalUnprocessed(bytes32 propID);
_ChallengeSucceeded(uint challengeID);
_ChallengeFailed(uint challengeID);
Current:
event _ReparameterizationProposal(address proposer, string name, uint value, bytes32 propID);
event _NewChallenge(address challenger, bytes32 propID, uint pollID);
Proposed:
event _ReparameterizationProposal(address proposer, string name, uint value, bytes32 propID);
event _NewChallenge(address challenger, bytes32 propID, uint pollID);
+ event _ProposalAccepted(bytes32 propID);
+ event _ProposalUnprocessed(bytes32 propID);
+ event _ChallengeSucceeded(uint challengeID);
+ event _ChallengeFailed(uint challengeID);
+ event _RewardClaimed(address voter, uint challengeID, uint reward);
PollCreated
: change args to commitEndDate
, revealEndDate
Add: underscores for convention
Experimental:
VoteRevealed
: change to (voter, pollID, votesFor, votesAgainst)
Current:
event VoteCommitted(address voter, uint pollID, uint numTokens);
event VoteRevealed(address voter, uint pollID, uint numTokens, uint choice);
event PollCreated(uint voteQuorum, uint commitDuration, uint revealDuration, uint pollID);
event VotingRightsGranted(address voter, uint numTokens);
event VotingRightsWithdrawn(address voter, uint numTokens);
Proposed:
_ event _VoteCommitted(address voter, uint pollID, uint numTokens);
* event _VoteRevealed(address voter, uint pollID, uint votesFor, uint votesAgainst);
* event _PollCreated(uint voteQuorum, uint commitEndDate, uint revealEndDate, uint pollID);
_ event _VotingRightsGranted(address voter, uint numTokens);
_ event _VotingRightsWithdrawn(address voter, uint numTokens);
Currently in the exit
function the event triggered is _ListingRemoved
. Since _ListingRemoved
is also called when the domain is challenged out, creating either an additional event or replacing the current event int the exit
would be a nice to have. Something like _ListingWithdrawn(_listingHash)
.
In development it will help differentiate whether the listing was voted out or withdrawn.
It can be confusing to see what "reward" is referring to
determineReward
CALL: get the number of tokens for the winning party of a challenge (Applicant or Challenger)
getChallengeWinnerReward
voterReward
CALL: get the number of tokens for an individual Voter who voted in the majority_bloc
of a challenge.
getVoterReward
claimReward
SEND TX: transfer voterReward
of msg.sender
to msg.sender
claimVoterReward
tokenClaims
CALL: see whether a Voter has tokens available to claim (i.e. did voter call claimReward
yet)
doesVoterHaveRewardToClaim
, doesVoterHaveTokenClaims
Line 134 in db40cd3
The intent of this line is to prevent people from making withdrawals that would leave them subject to touch-and-removes. It does that.
A side effect is that while a challenge is active, tokens cannot be withdrawn which would leave the unstaked deposit less tnan minDeposit, which is actually unnecessary. If the listing loses the challenge they will go away, and if they win they will necessarily be left with at least minDeposit tokens unlocked since they get their deposit amount unlocked plus their winnings from the challenge loser. Since multiple challenges cannot be in flight against a listing at the same time, it is okay if during a challenge a user withdraws their entire unstaked deposit.
not just in the audit findings
I copied this code from plcr factory. In this implementation, this chunk of code couldn't actually be possible, right? because the voting and parameterizer contracts need to know the token address when they are instantiated. this function creates a token but the voting and parameterizer contracts wont accept it.
@skmgoldin we're planning to put PLCRFactory on ethpm and consume it here right?
correct me if im wrong, but it seems the only place we need the option create a new token is in the plcr factory
Add two events to Parameterizer.sol to track when a proposal challenge has succeeded or failed like Registry.sol
event _ChallengeSucceeded(uint challengeID)
event _ChallengeFailed(uint challengeID)
Add event to Parameterizer.sol to track when a proposal has been accepted similar to Registry.sol NewListingWhitelisted.
event _ProposalAccepted(bytes32 propID) - when proposal is accepted with no challenge
event _ProposalUnprocessed(bytes32 propID) - when proposal processBy date has passed
using node 10 an error was reported
The problem is with with truffle": "4.1.11
> truffle install
TypeError: Cannot read property '1' of null
at module.exports (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/utils/args.js:9:1)
at processExports (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/index.js:61:1)
at processExports (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/index.js:82:1)
at /Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/index.js:137:1
at Array.forEach (<anonymous>)
at processExports (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/index.js:132:4)
at module.exports (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/promisify-node/index.js:164:1)
at Object.<anonymous> (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/ethpm/lib/installer.js:2:1)
at __webpack_require__ (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/webpack/bootstrap b4601922d6f11f8bff0b:19:1)
at Object.<anonymous> (/Users/doh/repo/tcr/node_modules/truffle/build/webpack:/~/ethpm/index.js:2:1)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install: `truffle install`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/doh/.npm/_logs/2018-09-10T07_37_42_278Z-debug.logtruffle": "4.1.11```
Sorry if this is not the right place to ask, but I was wondering to what extend have the smart contracts been audited?
100 - get("pDispensationPct")
can underflow.
Recommendation
Use SafeMath for arithmetic, and assert that these calculations succeed in processProposal so proposals can't get the contract in a state where the arithmetic for new challenges will revert every transaction.
truffle exec scripts/deploy_proxies.js failed silently.
Using network 'development'.
RegistryFactory: 0x23e84570f7b28ad0d5e5545d398009194a8734f3
Deploying proxy contracts...
did a truffle debug and got the following error
Parameterizer.sol:
248: @param _challengeID the challenge ID to claim tokens for
249: */
250: function claimReward(uint _challengeID) public {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
debug(development:0xb38b118c...)>
Transaction halted with a RUNTIME ERROR.
This is likely due to an intentional halting expression, like assert(), require() or revert(). It can also be due to out-of-gas exceptions. Please inspect your transaction parameters and contract code to determine the meaning of this error.
Add _RewardClaimed(msg.sender, _challengeID, reward) to Parameterizer.sol claimVoterReward function similiar to Registry.sol event https://github.com/skmgoldin/tcr/blob/master/contracts/Registry.sol#L281
This helps distinguish when a listing has been touched and removed instead of just a listing has been removed.
Presently, Registry.sol
imports Parameterizer.sol
in its entirety, but only calls the function get(string _name)
.
You can save gas and storage space by using a ParameterizerInterface.sol
in Registry
interface ParameterizerInterface {
function get(string _name) public view returns (uint value);
}
npm install
runs truffle install
, ahh!
let's see what happens when we change this scary number
When a user submits a proposal and it goes unchallenged, the user does not get their token back after the processProposal
function is invoked.
In the processProposal
function in the first condition if(canBeSet(_propID))
the value is being set but the token is never transferred back to the owner.
There should be a token.transfer
within this condition.
Tokens are being locked in contract.
https://github.com/skmgoldin/tcr/blob/master/contracts/Parameterizer.sol#L221
Hi guys,
I had an issue when tried to debug transaction with the latest truffle 4.1.13
Gathering transaction data...
redux-saga error: uncaught at session.saga
at session.saga
at recordInstance
TypeError: Cannot destructure property `context` of 'undefined' or 'null'.
at Object.addInstance (/Users/theanht1/.nvm/versions/node/v8.7.0/lib/node_modules/truffle/build/webpack:/packages/truffle-debugger/dist/debugger.js:4523:18)
at addInstance.next (<anonymous>)
at recordInstance (/Users/theanht1/.nvm/versions/node/v8.7.0/lib/node_modules/truffle/build/webpack:/packages/truffle-debugger/dist/debugger.js:3452:1)
at recordInstance.next (<anonymous>)
at next (/Users/theanht1/.nvm/versions/node/v8.7.0/lib/node_modules/truffle/build/webpack:/~/redux-saga/es/internal/proc.js:311:1)
at currCb (/Users/theanht1/.nvm/versions/node/v8.7.0/lib/node_modules/truffle/build/webpack:/~/redux-saga/es/internal/proc.js:388:1)
at runSelectEffect (/Users/theanht1/.nvm/versions/node/v8.7.0/lib/node_modules/truffle/build/webpack:/~/redux-saga/es/internal/proc.js:699:1)
I used scripts/deploy_proxies.js
to create contracts. I guess this error caused by the factory pattern because I can debug normally with tcr 1.0 (without factory). Have you ever faced it?
In the Registry.sol contract there is a claimVoterReward
function which would be very useful in Parameterizer.sol for determining whether or not the reward has been claimed.
Using web3, it is not possible to retrieve the voterCanClaimReward
mapping from the challenges
data. So for UI development it cannot be determined if the rewards have been claimed aside from the user's account receiving the token.
There is no problem claiming rewards, there is just no way using web3 to determine whether it's been claimed already.
I'm presuming that the name of a registry will be a significant coordination point for token holders and users of a registry. Reparameterizing the name to better reflect the values of the TCR as the values settle and shift over time may be valuable.
VOTE_QUORUM was misnamed in the paper and is misnamed in this implementation. What VOTE_QUORUM describes has nothing to do with the normal dictionary definition of a quorum. As-implemented (and as described in the paper), VOTE_QUORUM is the share of tokens voting in favor of a candidate (either a listing or applicant) for that candidate to prevail in a challenge. So it describes either the majority or minority of support necessary for the candidate to become/remain whitelisted.
Proposed new name: CANDIDATE_SUCCESS_THRESHOLD
As of f89eb89, coverage output looks like this:
--------------------|----------|----------|----------|----------|----------------|
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
--------------------|----------|----------|----------|----------|----------------|
contracts/ | 97.36 | 66.87 | 98.18 | 97.04 | |
PLCRVoting.sol | 96.2 | 55.17 | 95.45 | 95.24 |102,103,105,370 |
Parameterizer.sol | 98.82 | 68.18 | 100 | 98.85 | 223 |
Registry.sol | 97.03 | 76.56 | 100 | 96.97 | 136,137,139 |
--------------------|----------|----------|----------|----------|----------------|
All files | 97.36 | 66.87 | 98.18 | 97.04 | |
--------------------|----------|----------|----------|----------|----------------|
% Stmts
should be at least 95% and % Branch
should be at least 85%. These targets currently exclude tests for where token transfers fail. They also exclude PLCRVoting.sol
, since that will be consumed from EPM in the future, and the tests really belong in the PLCRVoting
repository.
Non-covered test cases to be added:
proposeRarameterization
challengeReparameterization
processProposal
dispensationPct
is greater than 100pDispensationPct
is greater than 100claimReward
resolveChallenge
processBy
date has passed
apply
minDeposit
withdraw
exit
challenge
determineReward
It would reduce cognitive overhead for users if the exitPeriodLen was just the exitTimeDelay value. I don't think it needs to be overcomplicated. We already have a bunch of complicated parameter names.
Line 279 in aca98f6
The variable name is confusing. Something like voterRewardClaimed is clearer.
By setting deployToken
to false you can specify an address for any token whose API is a superset of ERC20. Can this be made even more modular in the migrations?
PLCRVoting determines the winner with (100 * poll.votesFor) > (poll.voteQuorum * (poll.votesFor + poll.votesAgainst))
. As a result, polls resolve as failing when they expire with no votes. This is probably the right behavior, but it's surprising.
Recommendation
Document this behavior.
e.g. the as
function can be confusing for first-timers
Not sure if I'm missing something obvious, but config.json
doesn't seem to be actually used anywhere other than in tests. However, the README seems to imply that config.json
actually does something:
To deploy your own TCR, first open up `conf/config.json`...
On that note: there isn't actually a way to automatically deploy a TCR, is there? When I run truffle migrate
, only the factories are deployed. Wouldn't I have to manually call the factory functions to create TCRs?
This is all good; it's just that the README makes it sound like there's a way to just edit config.json
and have a TCR automatically deployed, but that doesn't seem to be the case.
The purpose of the _data
string in the apply
function is for applicants to be able to furnish arbitrarily complex data related to their application. For example, an IPFS hash which resolves to:
{
"name": "harvard",
"average_student_debt": "10000 USD",
"weather_in_locale": "poor"
}
In the TCR of top colleges, the identifier string here is that listed under the "name" property. The _listingHash
should be keccak256("harvard")
, and token-holders should strongly suspect they are being scammed if the supplied name does not match the supplied _listingHash
.
A separate example could be:
{
"domain": "consensys.net",
"scamvoid_score": "high"
}
Here, the "domain" is the identifier string.
User interfaces should be able to flag to users when an identifier in a _data
object does not match the provided _listingHash
. This will require some way of detecting what the identifier property is. Perhaps for the purposes of user interfaces, by convention, an 'identifier' property in the _data
object could be required?
Though there is an ethpm.json
file in the repo, the tcr
package is not published to the registry and cannot be installed on another project via truffle install tcr
Registry.sol:186:13: Warning: Invoking events without "emit" prefix is deprecated.
_TouchAndRemoved(_listingHash);
^----------------------------^
Parameterizer.sol:155:5: Warning: Invoking events without "emit" prefix is deprecated.
_ReparameterizationProposal(_name, _value, propID, deposit, proposals[propID].appExpiry, msg.sender);
Easily fixed by adding the emit
prefix to all events.
Registry.sol:214:29: Warning: Use of the "var" keyword is deprecated.
var (commitEndDate, revealEndDate,) = voting.pollMap(pollID);
Parameterizer.sol:189:25: Warning: Use of the "var" keyword is deprecated.
var (commitEndDate, revealEndDate,) = voting.pollMap(pollID);
Not sure how to go about this one. Trying to store the result to a Poll
struct gives a TypeError
Parameterizer.sol:189:5: TypeError: Too many components (5) in value for variable assignment (1 needed).
PLCRVoting.Poll memory poll = voting.pollMap(pollID);
^--------------------------------------------------^
Registry.sol:176:9: Warning: This declaration shadows an existing declaration.
uint deposit = parameterizer.get("minDeposit");
^----------^
Registry.sol:115:5: The shadowed declaration is here:
function deposit(bytes32 _listingHash, uint _amount) external {
^ (Relevant source part starts here and spans across multiple lines).
Can be fixed by changing uint deposit
to uint minDeposit
(seems more descriptive too)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.