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

[RFC] Upgradeable contract components #726

Merged
merged 8 commits into from
May 31, 2019
Merged

Conversation

eth-r
Copy link
Contributor

@eth-r eth-r commented Apr 8, 2019

closes #725

By splitting work contracts to front-end and back-end components, secure upgrades can be achieved without excessive headache. Immutable back-end contracts maintain system integrity towards stakers, while front-end contracts can abstract and delegate over multiple back-ends to provide a smooth customer experience.

Responses to questions in #133 :

What conditions trigger a “hard fork”-style whole-system upgrade?

Only upgrades to the token contract require a full "hard fork". Upgrades to staking contracts or work contracts can be gracefully accommodated.

In individual work contracts, the front-end upgrade rules determine this. If front-end code is immutable, only upgradeable with new backends, any upgrades that require changing the rules of the front-end necessitate a hard fork on the specific contract. If eternal storage is used, a hard fork is required if the storage needs changing.

What does a whole-system upgrade consist of? e.g., what happens to the relay and its relay groups? [1] What happens to keeps?

With a frontend hard fork, the relay would need to be restarted with some initial entry (possibly from the entries produced on the previous frontend). If the entries are kept in eternal storage, the relay could continue directly.

Groups would be backend-specific, so whether groups are kept depends on whether old backends are kept.

What do softer upgrades look like (e.g., client upgrades)?

Deploying a new backend contract, adding it to the frontend's backends, waiting for the frontend to start creating groups on the new backend.

[1]- One concrete question for the relay is, say we want to change the curve on which we run BLS key generation and signing. How do we manage this? In particular, do we require all active relay groups to regenerate their keys post-upgrade? Do we support two curves in parallel? Do we drop all groups and restart the relay? Working out some of the implications of these decisions would be good. Several of them will likely apply to other places as well (e.g., Threshold ECDSA likely will have similar questions about crypto upgrades).

The upgrade scheme provides an elegant way to support curves in parallel. Changing the curve would only be a matter of deploying a new backend contract and staking clients that support it; no restart required. From the perspective of the customers, only use-cases that involve specifically validating entries would be impacted as they would need to accommodate the new curve. Similar solutions are likely viable with tBTC as well.

Copy link

@madxor madxor left a comment

Choose a reason for hiding this comment

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

Slick proposal. One comment.

When separating front-end and back-end we need to consider the increased cost of inter-contract communication. I would propose to create a research PR that would be devoted to estimating how less gas-efficient we are going to be.

@ngrinkevich
Copy link
Contributor

Slick proposal. One comment.
When separating front-end and back-end we need to consider the increased cost of inter-contract communication. I would propose to create a research PR that would be devoted to estimating how less gas-efficient we are going to be.

Slick proposal indeed 👌
Regarding inter-contract communication from what I see we already have two in requestRelayEntry and relayEntry and from my understanding refactoring as backend/frontend we're still no more than two of inter-contract transactions with group_created and entry_created callbacks.
But of course main benefit is that we only need one place for stakers to authorize token transfers (the backend)

that is stripped to the minimum necessary
for security and correct incentives.

Each backend contract is associated with one or more front-end contracts
Copy link
Contributor

Choose a reason for hiding this comment

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

@eth-r could you confirm pls, shouldn't backend have only one hardcoded frontend address?
For example if there is a list of frontends, how would it work, the backend would do callbacks (group_created, entry_created) on all of the addresses in the list in a loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's multiple frontends (for example, one frontend might be customer-facing and another for internal usage so Keep Org can fast-track past any request queues, price discrimination etc. when using the beacon for tBTC or anything else) the backend might want to let all frontends know about changes to the number of groups, but otherwise track which request came from which frontend and only call entry_created on that one.

It might look like: (frontends A, B and C)

  1. A requests entry X
    • the request is stored along with the requesting frontend
  2. B requests new group
    • the request is stored, but requesting frontend is not strictly necessary to store
  3. backend returns entry X and update to number of groups due to expiration:
    • call group_created on B and C
    • call entry_created on A
  4. C requests entry Y
    • the request is stored, along with the fact that it was requested by C
  5. backend finishes DKG in response to step (2), so it updates the number of groups:
    • call group_created on A, B and C
  6. backend returns entry Y and new groups:
    • call group_created on A and B
    • call entry_created on C

ngrinkevich
ngrinkevich previously approved these changes Apr 23, 2019
Copy link
Contributor

@ngrinkevich ngrinkevich left a comment

Choose a reason for hiding this comment

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

Looks good to me! I think all the aspects have been described very well, haven't found any major issues.

Copy link
Member

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

I like it but I am not sure if we should split logic between frontend and backend. Shouldn't we rather keep entire logic in one place accessed via proxy and separate data storage for easier upgradeability?

What's the main benefit of having frontend/backend separately instead of just
proxy->backend->storage ?

docs/rfc/rfc-9-upgradeable-contract-components.adoc Outdated Show resolved Hide resolved
docs/rfc/rfc-9-upgradeable-contract-components.adoc Outdated Show resolved Hide resolved
docs/rfc/rfc-9-upgradeable-contract-components.adoc Outdated Show resolved Hide resolved
docs/rfc/rfc-9-upgradeable-contract-components.adoc Outdated Show resolved Hide resolved
@ngrinkevich
Copy link
Contributor

What's the main benefit of having frontend/backend separately instead of just
proxy->backend->storage ?

main benefit we can keep eternal storage approach for the frontend, but not for the backend where that approach is not ideal (all stakers tokens are lost in case of compromised upgrade key)

@mhluongo
Copy link
Member

Took me a bit to grok and come around, but this is an excellent upgrade approach to respect staker security / sovereignty. I think it's worth proposing as a community standard or at least a blog post @cryptojane55

@mhluongo
Copy link
Member

It feels like this is close. @eth-r can you address the open comments (not to change the structure of what you're proposing, just as points of clarification in the RFC) and @pdyraga can give it another review?

@mhluongo
Copy link
Member

Prior discussion in RFC 4- merged the RFC as superseded by the work here. Would be good to get a "Supersedes RFC 4" type note in this one and get it merged. @pdyraga any other concerns?

@pdyraga
Copy link
Member

pdyraga commented May 28, 2019

I think it looks very good but we need to clarify a few aspects to avoid misunderstandings (terminology) and to make this patter sustainable long-term.

To summarize:

keeping its own state on security-critical data.
The operator contracts provide simplified functionality
that is stripped to the minimum necessary
for security and correct incentives.
Copy link
Member

Choose a reason for hiding this comment

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

💯 !

@pdyraga
Copy link
Member

pdyraga commented May 29, 2019

Just one question left and I will light the green torch.

@mhluongo @ngrinkevich Some terminology has been changed and some aspects clarified/detailed so maybe you want to take another look before we merge?

@ngrinkevich
Copy link
Contributor

Looks good to me! Operator/Service contract names sound better indeed 👍

ngrinkevich
ngrinkevich previously approved these changes May 29, 2019
When security-critical functionality cannot be contained within operator
contracts, service contracts or applications have to be trusted. Include the
basics of dealing with this situation in RFC 9.
@eth-r
Copy link
Contributor Author

eth-r commented May 30, 2019

I'm reasonably happy with the current state of this RFC so it's ready for final review

@eth-r
Copy link
Contributor Author

eth-r commented May 30, 2019

And to clarify the question from @pdyraga, exact method for dispatch between versions is out of scope but it should be gradual from 0% (immediately when the new version is first published and hasn't been authorized by any stakers yet) to 100% (when the new version is sufficiently established that almost all stakers reliably operate using it)

@pdyraga
Copy link
Member

pdyraga commented May 31, 2019

Let's do it!

@pdyraga pdyraga merged commit 25fc833 into master May 31, 2019
@pdyraga pdyraga deleted the rfc-component-upgrades branch May 31, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify contract upgrade scheme
5 participants