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

sig nonce usecase #44

Open
nichonien opened this issue Nov 29, 2021 · 8 comments
Open

sig nonce usecase #44

nichonien opened this issue Nov 29, 2021 · 8 comments

Comments

@nichonien
Copy link
Contributor

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.

@mirceanis
Copy link
Contributor

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.

@nichonien
Copy link
Contributor Author

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?

@mirceanis
Copy link
Contributor

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.

@nichonien
Copy link
Contributor Author

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.

@mirceanis
Copy link
Contributor

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

@nichonien
Copy link
Contributor Author

nichonien commented Nov 30, 2021

@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!

@mirceanis
Copy link
Contributor

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants