AdChain / AdChainRegistry

adChain registry smart contracts
Apache License 2.0
30 stars 10 forks source link

Re-implement parameterizer #28

Closed skmgoldin closed 7 years ago

skmgoldin commented 7 years ago

Parameterization is considered an unsolved problem in token-curated registries; there is no formal purity in any of the proposed solutions, and there are known attacks. The formal weakness in TCR parameterizers is that they may be put into “stuck states”, EG, the COMMIT_STAGE_LEN parameter could be set to be very long, effectively breaking challenges. The AdChain parameterizer is an attempt at a practically safe if formally-impure cryptosystem which retains the registry's property of being completely decentralized. In this design, financially-motivated token holders are incentivized to be diligent and not allow “breaking” proposals to be accepted. There is no safety net after that, however.

Mechanically, the AdChain parameterizer implements much of the same logic as the registry itself. The main differences are that instead of Listings we use ParamProposals, and deposits are simply returned if a proposal is accepted without a challenge. This is to say, set parameters are not “challenged” the way listings are. Instead, a new proposal is made and either accepted without challenge or, if challenged, perhaps accepted or perhaps rejected. When a new proposal is made, AdToken is at stake and challenges are thereby incentivized.

The AdChain parameterizer has its own parameters separate from those of the registry itself. The parameters prepended with p such as pMinDeposit are parameters of the parameterizer itself. This is a safety feature: because re-parameterizations are potentially dangerous operations, logically all of the working parameters should be set significantly higher than in the registry itself to mitigate spam and un-serious actors from putting the system at risk.

ParamProposals have a processBy date hardcoded to seven days plus the duration of the apply, reveal and commit stages. This is another safety feature which basically requires that ParamProposals be processed within some reasonable period of time after which they cannot be set no matter what (though challenges on them can be resolved for the purposes of disbursing tokens). The reason for this is to prevent an attacker from securing an acceptance vote on a re-parameterization which makes sense at time a, but which the attacker sits on until time b to deploy when that re-parameterization may not make sense. This is implemented here.

Additional safety features include disallowing duplicate re-parameterizations, meaning that if param x is set to a, a proposal to set x to a will be automatically rejected. Furthermore, if a proposal exists to set x to b, follow-on proposals to do the same will be automatically rejected until the first is processed. This is just to reduce spamming, as well as cognitive overhead for voters (“but I thought we rejected that proposal?”).

Not absolutely everything discussed in this PR is actually fully implemented yet, but I would estimate 80% of it is. The test suite is not complete either and it has not been user tested, so it probably has bugs. Furthermore, there is logic related to challenges which is duplicated between Registry.sol and Parameterizer.sol. All of these will be addressed in future pull requests, but I want to merge this to master so we can ship it to the application team by middle of the week. The existing parameterizer is unused by them, so it won’t break anything.

skmgoldin commented 7 years ago

Thanks for the code review, will update.