search

urbit / azimuth

General-purpose PKI on Ethereum
MIT License
134 stars 36 forks source link

Add CI, don't run nondeterministically-failing tests in CI #31

Closed jtobin closed 4 years ago

jtobin commented 4 years ago

This doesn't include much:

I also took the opportunity to fix up the artifacts.require path syntax in the migration and test scripts, and to ditch mocha, which wasn't being used at all. Also the lockfile wasn't being tracked via git, but probably should be, so I removed that from .gitignore.

Related to fixing #13, I've started some work on updating the contracts (and everything else -- see the jt/modernise branch) with a sub-goal of finally resolving that issue, but just plan on picking at that stuff whenever I'm in the mood over the next indeterminate period of time. In the meantime: it's important to have some kind of useful CI running without needing to babysit it too much, so absent being able to make the ERC721 compatibility tests pass consistently, I don't think we should include them in CI.

jtobin commented 4 years ago

Uh... probably don't do this? Nothing but the Ecliptic can be upgraded, and even there you want to keep a copy of the old version at Ecliptic1.sol or w/e. The contracts here should match their on-chain versions, and those just can't change.

Yeah, I figured we would only propose to merge that work conjointly with a proposal to the senate to upgrade the ecliptic (the old version would remain in the git history, of course).

For the others -- I just sort of lazily intuited that new versions of at least most could be deployed and ownership/permissions/etc. reassigned as necessary, appealing to the senate as needed to approve the required changes. Is that not true?

It seems like it would be nice to be able to use more recent solc, openzeppelin-contracts, etc. versions for potential security/gas cost improvements, but perhaps muh endeavour is misguided..

jtobin commented 4 years ago

(Will merge this one in the meantime anyway.)

Fang- commented 4 years ago

For the others -- I just sort of lazily intuited that new versions of at least most could be deployed and ownership/permissions/etc. reassigned as necessary, appealing to the senate as needed to approve the required changes. Is that not true?

This is not really true. Azimuth.sol is an exemplary example of why: there's a lot of data in there (keys, ownership, etc) that a newly initialized contract wouldn't have. Porting the data over into a new contract is theoretically possible, but you really don't want to do it. It's very non-trivial implementation-wise, has potentially enormous gas/ETH costs, and breaks promises around having a persistent data storage contract.
All of that is slightly more easily overcome for the other contracts, but I wouldn't do the work here if we aren't otherwise planning significant updates to those.

jtobin commented 4 years ago

Azimuth.sol is an exemplary example of why

Right -- Azimuth is a big potential exception. It's mainly the others I'm thinking of. Regardless I definitely wouldn't imagine this becoming a major line of work any time soon, agreed.