-
Notifications
You must be signed in to change notification settings - Fork 76
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
RFC 4: Secure upgrades for contracts operating staked balances #446
Conversation
Proposal to use alternative upgrading approach that is safe for critically important operations dealing with staked balances.
de07c3b
to
63f693c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left several suggestions + some thoughts. This is a good overview, but I think we need a touch more detail in the RFC.
implementation as a new contract. Address of this contract must be | ||
re-authorized by a staker. Stakers do so by calling authorize(address) | ||
method on a staking contract. To further secure the network only official | ||
Keep org contracts addresses can be authorized. The metacontract list of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be “a client should only authorize official contract addresses from the Keep organization”. Otherwise someone who would have access to upgrade Keep contracts presumably also has access to update a global contract-based list of authorizable contracts.
I'm thinking the contract list update should be more of a multifactor thing… e.g. it's listed in the metacontract and you've seen it confirmed two or more Keybase-proven channels or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on whether we want client-side or on-chain verification. Per the flowdock discussion, if we have separate upgrade keys for each service and its list of authorizable contract addresses, and master keys for rekeying the upgrade keys subject to staker approval, it should be reasonably secure:
the metacontract has upgrade(contract, address)
and panic_button(contract)
which check that the signature matches the upgrade keys for that contract, and rekey(contract, pubkey)
which then waits for a supermajority of stakers to approve the new upgrade keys
If the client authorises a non-Keep org address, it would present a risk of unexpected behaviour even if the only functionality exposed to the authorised contract is slash(staker, penalty)
and slash_and_transfer(staker, penalty, tattletale, reward), reward < penalty
. With client-side multi factor validation, this can't be enforced on-chain.
Amongst other things, these contracts only allow token transfers to be | ||
executed by the token holders. Since token staking functionality might | ||
need upgrading, it was decided to simply issue and deploy a new contract | ||
and advise stakers to migrate their balances to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With extended unstaking time, this may prove inconvenient in use; however, it's the most secure method and the staking contract can be completely upgrade-agnostic.
There's a risk of a "stakepocalypse" if a new staking contract is released and everybody rushes to unstake so they can migrate immediately.
However this means the work requested during the unstaking time gets distributed over those who stay in the old contract, making it disproportionately profitable to stay and unstake+restake later when the demand moves to the new contract.
Thus the stakepocalypse won't really happen, and the stakes will probably be migrated across over 2 * unstaking_time
time, by:
- unstaking a fraction of one's tokens, keeping the rest on the old contract
- after the unstaked tokens are freed, staking them on the new contract
- unstaking the rest from the old contract
- staking the rest on the new contract
If the demand for work on the old contract within the time [T .. (T + unstaking_time)]
is A
and demand on the new contract from [(T + unstaking_time) .. (T + 2*unstaking_time)]
is B = pA
, and the fraction of stakers who unstake is u
, the revenue increase one gains from not unstaking immediately is A / (1 - u)
and the opportunity cost is B / u
. This means that it's profitable to wait on unstaking if:
A / (1 - u) > B / u
(A * u) / ((1 - u) * u) > (B * (1 - u)) / (1 - u) * u)
A * u > B - B * u
(A + B) * u > B
u > B / (A + B)
It's probably easiest to just do 50/50; if everybody does it the income distribution doesn't change (correctly anticipating increased demand can improve on this unless everybody does it), and it can be done by always splitting one's tokens across two stakers even if no specific provisions are made for partial unstaking.
The contracts with `authorize()` method should also include a "panic button" | ||
- a method that cancels all authorizations in case of emergency. This | ||
should be restricted to be called by a Keep organization account used to | ||
deploy this contract. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make the panic button act on a specific work contract, effectively freezing its functionality but protecting all (remaining) stakes. This makes it significantly more useful as the panic button doesn't have to stop absolutely everything, but only the unsafe contracts.
Making panic buttons work against staking contracts, however, is rather difficult and probably not worth the effort.
method on a staking contract. A client should only authorize official | ||
contract addresses from the metacontract list maintained by Keep | ||
organization along with the confirmation by two or more Keybase-proven | ||
channels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can optionally be enforced in the staking contract, if Keep Org maintains a contract that tracks authorized versions.
=== Implementation | ||
|
||
Each new contract that does "slashing/reward" changes on staked token | ||
balances must be a non-upgradable and non-ownable one to minimize attack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to make a layercake of:
- "back-end": the operator-facing work contracts, upgraded atomically and securely
- "front-end": an user-facing interface contract with its own, possibly more liberal upgrade process, which offloads work to the work contracts and does migrations across them
The front-end can safely implement eternal storage as long as the work contracts don't grant it dangerous privileges.
For example, in the random beacon the front-end contract could implement the beacon itself and its pricing/request process; the back-ends would handle signing the input they receive from the front-end, and creating new groups when the front-end asks them to.
When a request comes in:
- the frontend randomly selects which backend contract serves it, based on e.g. the number of groups each backend has active
- the frontend sends the request to the selected backend, along with payment
- the backend selects which of its groups will serve the request
- the selected group produces the signature
- the backend rewards (or punishes) signers and sends the signature to the frontend
- the frontend processes the signature and responds to the requester; the frontend might also request the creation of a new signing group in the most recent backend contract (which may differ from the contract that produced the entry), and send the DKG payment to it
This way the interface presented to the customers can be unchanging while stakers' funds remain perfectly secure against unauthorized upgrades; in the worst-case scenario the frontend breaks, making the system unable to do useful work but not causing stakes to be stolen or lost. The exact API separation needs to be architected individually for each usecase, however.
Is this superseded by RFC 9 / #726? If so, can we link back to the discussion from the new RFC and close? |
If we do this we should probably also renumber RFC 9 to RFC 4 as there will be no actual RFC 4. |
I suppose the alternative is merge with a deprecation notice and a back-link? We've been referring to RFC 9 as RFC 9 so changing the numbering now wouldn't be awesome |
Could also just be a not-accepted status via an admonition up top. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering the the RFC is superseded, this LGTM
Proposal to use alternative upgrading approach that is safe for critically important operations dealing with staked balances.