Contract upgrade proposal 001

[Fang-]

As previously discussed in the galaxies group. Background and details underneath the fold.

Details of the proposal submitted to the senate on 2021-01-18.

Honored members and representatives of the Galactic Senate, The Azimuth suite of contracts has been on the Ethereum blockchain for over two years without any real trouble. As such, the contracts have had no strong need to be upgraded yet. Now with the milestone votes behind us, and an on-Urbit forum facilitating discussion, the time seems right to try our hand at one of these fabled upgrade proposals. As a refresher, "Azimuth" actually refers to two contracts: [Azimuth](https://github.com/urbit/azimuth/blob/master/contracts/Azimuth.sol), the database contract, and [Ecliptic](https://github.com/urbit/azimuth/blob/master/contracts/Ecliptic.sol), the logic contract. In an upgrade proposal, the Senate votes on a replacement for the Ecliptic. The data and database format must remain the same, but the rules by which we interact with it may change arbitrarily. Note that I am intentionally excluding big or potentially controversial changes from this upgrade. The smaller scope seems appropriate to a first upgrade. Please let me know your thoughts about the proposed changes below. (For questions about the contracts and their upgrade mechanism more broadly, I would be happy to field those in the general chat, or in a separate discussion thread.) ## Changelist Sometime this quarter, I intend to open a vote on an upgrade of the Ecliptic contract (the "business logic" of Azimuth) containing the following changes: - Fixed ERC721 compatibility - Self-modifying proxies - Upgraded Claims contract Let us go over these in some more detail. ### Fixed ERC721 compatibility The contract definition of the Ecliptic includes various functions and events that make it conform to the ERC721 (non-fungible token) standard. However, two events have [ever so slightly non-conforming](https://github.com/urbit/azimuth/issues/8) descriptions. This causes Ethereum explorers like Etherscan to incorrectly recognize Azimuth as ERC20 (fungible token), rather than ERC721. The proposed change simply modifies the event descriptions to accurately match the ERC721 definition. This does not affect any functionality within the contract itself. ### Self-modifying proxies Owners of Azimuth assets are allowed to configure "proxy addresses" for them: Ethereum addresses allowed to act "as" the owner, but only for a subset of operations. For example, setting the voting proxy will let you vote using that address in addition to your ownership address. This is useful for keeping the ownership keys in very cold storage, while still letting you perform lower-value operations. Currently, only the ownership address is allowed to change proxy addresses. Fairly early on after Azimuth's deployment, we realized it might be nice for proxies to be able to change _themselves_. This means that, in addition to being able to vote, your voting proxy would be able to assign a new voting proxy. This would let you rotate your proxy keys without needing to take your ownership keys out of cold storage. ### Upgraded Claims contract [The Claims contract](https://github.com/urbit/azimuth/blob/master/contracts/Claims.sol) lets asset managers associate various attestations with their identity. For example, an Ethereum address to send donations to, or proving ownership of a Twitter account. For ease of use by off-chain tools and services, the contract emits events (notifications) whenever any claims are updated. However, the current version of the contract has a bug where it does not emit events when all claims are removed at once (`clearClaims()`). This "gap" in the event stream makes it much more difficult to write off-chain tools around this contract. Considering Claims has seen [practically no use](https://etherscan.io/address/0xe7e7f69b34d7d9bd8d61fb22c33b22708947971a#events) since it was first deployed, it should not be a problem to simply start over fresh with a new contract that contains [the fix](https://github.com/urbit/azimuth/pull/32) for the described bug. The reason we must do an Ecliptic upgrade to switch to a new Claims contract, is that it is tied in with the logic for transferring an asset. If the asset is flagged for "reset" (common when transferring to/from someone other than yourself), its configured proxies, networking keys _and claims_ are cleared during transfer. As such, the Ecliptic will need to know about the address of the new Claims contract.

In addition to the proposed changes, this PR includes tools for locally testing Ecliptic upgrades against a fork of mainnet Ethereum. In order to verify that the proposed contract can be upgraded to without trouble:

1) check out this branch, npm install, 2) npm run fork-mainnet, wait for it to become ready, 3) in a separate terminal, npm run test-upgrade, 4) wait patiently for tests to pass.

If/when the proposed contract gets deployed to mainnet, I will post instructions for simulating the upgrade to the deployed upgrade target. This should behave identically to the locally deployed version, but doing a final check in that way before kicking off the vote seems prudent.

Tagging @philipcmonk for contract review, @jtobin for tooling review, and of course welcoming anyone to come forward with feedback.

[Fang-]

@jtobin good catches all around. I disabled timeouts on the tests, but also didn't run into them myself. We might be able to speed this up by using raw web3 to the real mainnet for gathering senate addresses during the test itself too (the gathering during startup is noticeably faster), but not sure if that's worth it.

[Fang-]

Nice catch @philipcmonk. Updated the contract logic, and added tests to check this.

Debated whether it should be checking the dossier also, in addition to the protocol and claim, but decided against it. While you can technically call addClaim with empty strings for both protocol and dossier, this is semantically not a sane thing to do, and the contract doesn't deal with it elegantly in the individual removal case. Perhaps we should forbid that explicitly, requiring either claim or protocol to contain a non-empty value ?

[Fang-]

Apologies for the long silence on this. As of this transaction, voting on the upgrade, targeting 0x9ef27de616154ff8b38893c59522b69c7ba8a81c as the new contract, has started. Votes can be cast within the next thirty days, or until an absolute majority is reached, whichever comes sooner.

The contract's source code can be viewed here and should match this PR.
Instructions for testing the upgrade can be found in the opening comment (you may or may not need to additionally npm install -g ganache-cli before step 2).
Bridge has been updated to support voting on upgrade proposals in a manner similar to document ones.

[Fang-]

Voting has concluded with 91 votes in favor, 0 against. As of this transaction, the upgrade has been applied. Thanks to all who showed up and participated! Merging now.