ethr-did-registry icon indicating copy to clipboard operation
ethr-did-registry copied to clipboard

sig nonce usecase

Open nichonien opened this issue 3 years ago • 8 comments

Hi Team,

I am a software developer from Energy Web and have couple of queries regarding EthereumDIDRegistry contract.

The current deployed EthereumDIDRegistry doesn't have sig nonce changes (PR). Is there any specific reason for it? The changes are quite old and it is neither merged to the master nor deployed. We are moving to production and want to confirm which version of the contract should we move ahead with.

Also, would it make sense to have an Upgradable EthereumDIDRegistry contract?

Apologies If I am not using the right channel / template for such queries.

nichonien avatar Nov 29 '21 04:11 nichonien

This is the right channel, thank you for reporting this. You are right, that fix has not been deployed, so if you are using that feature it makes sense to use a more recent version.

The frontend library ethr-did-resolver will work with a new version too as long as you configure it with the proper registry address.

A general update of the contract seems necessary, not only for the bugs, but also because several patterns that it uses have been standardised as EIPs since the time it was initially written(meta tx, contract signatures). Also, the world of Ethereum is shifting to a rollup model and it is worth exploring how this registry could be used in the new model.

Making it upgradable raises questions of governance which are always hard to answer. This version was deliberately published without the option to upgrade.

Of course, this should not block further development, so if you have ideas or contributions to make, please do so.

mirceanis avatar Nov 29 '21 10:11 mirceanis

Thanks @mirceanis!

In addition to your statement

A general update of the contract seems necessary, not only for the bugs, but also because several patterns that it uses have been standardised as EIPs since the time it was initially written(meta tx, contract signatures). Also, the world of Ethereum is shifting to a rollup model and it is worth exploring how this registry could be used in the new model.

If we update the contract to the latest standards and specifications, will ethr-did-resolver continue to resolve DIDs ? or changes would be needed to incorporate the same?

nichonien avatar Nov 30 '21 06:11 nichonien

That depends on the changes. The old contracts are still there, and are immutable, so existing resolver implementations will continue to work.

Ideally, the upgrade would act as a proxy for the old data.

mirceanis avatar Nov 30 '21 07:11 mirceanis

Ok, so for now I guess we can at least make changes that are deprecated w.r.t current compiler versions. These changes won't change the function signatures or the meaning. So, revolver should be able to resolve DIDs without needing any change.

This would also enable EthereumDIDRegistry to be tested along with other contracts build on latest EIPs & compiler versions (fasten the development around DIDs ecosystem).

These changes could be (mostly around compiler version changes) :

if (owner != 0x0) { => if (owner != 0x0000000000000000000000000000000000000000) { [keccak256(delegateType)][delegate] => [keccak256(abi.encode(delegateType))][delegate] delegates[identity][keccak256(delegateType)][delegate] = now; => delegates[identity][keccak256(abi.encode(delegateType))][delegate] = block.timestamp;

byte(0x19), byte(0) => bytes1(0x19), bytes1(0) bytes value => bytes memory value

If this makes sense to you, then I can create a PR with all such minor changes atleast for now.

nichonien avatar Nov 30 '21 13:11 nichonien

yes, please go ahead with PRs. We can discuss individual changes there.

mirceanis avatar Nov 30 '21 14:11 mirceanis

@mirceanis I can't push my branch and create a PR (remote: Permission to uport-project/ethr-did-registry.git denied to nichonieni [email protected]). If you can grant access or suggest if there's any other way. Thanks!

nichonien avatar Nov 30 '21 15:11 nichonien

You can fork this repository and push to your fork. Then you can make the PR from the forked repository to the main repository.

mirceanis avatar Nov 30 '21 15:11 mirceanis

This issue and other requests made me realize that this contract's immutability is becoming an issue, so I started a governance topic for did:ethr on our discord: https://discord.gg/MTeTAwSYe7

mirceanis avatar Dec 07 '21 20:12 mirceanis