Core smart contract release process

Governance
#1

ICYMI the release process for the core smart contracts is published here.

The proposal is to always release all smart contract changes atomically. This will make keeping track of what contracts are deployed on a given network much simpler, as we can always ensure that the deployed contracts correspond to a specific commit hash. This also ensure that the contracts remain interoperable, as we don’t need to worry about keeping contracts interoperable with each other across different commits.

The implication here is that governance proposals to upgrade core smart contracts will be “all or nothing”, in the sense that each time there will be a single proposal to upgrade the smart contracts from commit hash X to commit hash Y, which will contain updates to all smart contracts that had code changes between X and Y. This means that if there are controversial changes, it will be easiest (though not strictly necessary) to commit to or reject those changes before they make it into master, rather than by voting down the governance proposal. Certainly rejecting those changes is still possible by rejecting the proposal, but it will mean those specific changes that were rejected would need to then be reverted on master, and a new governance proposal be made.

Some of us at cLabs are currently working on smart contract upgradability tooling in order to make sure deploying upgrades is safe and easy, so that Celo can maintain the development velocity needed in order to respond to user feedback and needs. We’re hoping to have this ready such that we can begin executing the first upgrade process within the next few weeks.

Some changes that are in master already that would be batched into the first upgrade include:

  1. Election.sol: A minor bug fix in which a value guaranteed to be zero was being subtracted unnecessarily
  2. Governance.sol: Return weight in getVoteRecord
    3: LockedGold.sol: Add additional require statements to confirm consistency of balances
  3. Exchange.sol: Short circuit to return zero when buy/sell amount is zero
  4. DowntimeSlasher.sol: Totally reimplemented, to allow downtime to be computed in pieces so as not to exceed the block gas limit.

I’d love to get more feedback on all of this, so that by the time the tooling is ready, we’re all on the same page with respect to how core contract upgrades should be done.

1 Like
#2

Copying Brian from stakevalley.com’s response in discord:

The atomic approach sounds good from a technological/release management perspective. At first glance, it raises these concerns/thoughts from a governance perspective:

  1. If a governance proposal has all changes from X to Y, it increases the risk of “pork barrel” changes. I.e. the larger the review/batch, the more difficult it will be for reviewers to spot problematic or dangerous changes. Smaller batches increases transparency & scrutiny IMO. This to me is the biggest risk of this change as it significantly increases the odds of a bad change making it in. How will we prevent large batches hiding small bad changes (intentional or unintentional)?

  2. If the guidance is to shift the debate to a pull request on master and make voting down a governance proposal more expensive, will this dis-incentivize the community from voting down problematic proposals b/c now rolling things back has additional cost?

  3. If the PR on master is where a lot of the debate will happen, we’ll need to make sure there are proper notifications/timelines/transparency for the community to weigh in. Just as we do for when new governance proposals are raised.

  4. For the feedback time on the PR, is there a way acceptance/rejection could be weighted by locked gold similar to how on-chain governance works? Not sure if this is needed, but figured I’d ask.

1 Like
#3

Thanks Brian! All great points. The release process was indeed designed to prioritize safety and velocity, and I agree that the atomic approach makes a governability tradeoff in favor of these priorities.

  1. I suppose it depends on when the reviewing happens. Even though the governance proposal will include a batch of changes, each individual change will still have been reviewed as individual PRs. Ideally, the actual code changes would get the most scrutiny here, before being proposed as on-chain changes. Furthermore, design changes (e.g. governance 2.0, support for multiple stable currencies in Exchange.sol) should be proposed as CIPs before any code is written, providing an additional forum for feedback. Finally, part of the release process includes reputable third party auditors carefully examining the changes to be deployed, providing an additional layer of protection beyond individual PR reviews and automated tests.
  2. Unfortunately I believe the effect you described would happen in some cases. If a core contract upgrade contains changes A and B, where A has universal support and B has middling support, desire to deploy A quickly may result in B being deployed, when in fact B may not have been deployed if the governance proposals were independent. I would hope that we would be able to catch controversial changes before they are written and merged into master via the CIP process, but it’s hard to guarantee this is the case. If B was met with broad opposition, I would hope that the governance proposal for A and B would still be voted down, as it will generally probably not be terribly hard to revert B in master and make a new proposal to adopt only A.
  3. I think it depends. It may be noisy to broadly flag every smart contract change (e.g. this minor change to add checks in LockedGold.sol may not be worthy of broad examination). Large design changes, IMO, should go through the CIP process (and be rejected if they don’t!), which would address your concern, I think.
  4. The governance contract allows for empty proposals, like was done for the cGLD/CELO rename. We could consider using those to vote on CIP adoption.
1 Like
#4

+1 to the idea of atomic updates and treating all core smart contracts as essentially a single application with single version.

My suggestion would be to establish predefined schedule for release frequency. So for example, we can decide to have quarterly releases or 6 month releases, but most important part being that it is predefined and predictable.

Without predefined schedule, some changes will always be too small to warrant running through the release process, and it can lead to accumulation of ton of tiny changes, until a huge important change actually needs to go out. This can lead to more difficult upgrade process and also more potential bugs.

Predefined frequency will probably fit well with overall review/audit/governance cycles too. Also it should work better for external contributors, without predefined schedule it can be hard to prioritize making contributions since you never know when your small change will actually get deployed in mainnet.

2 Likes
#5

Thanks Asa. All makes sense and addresses my concerns.