Add support for meta/signed transactions

[lleifermann]

This adds support for the EIP1056 "signed" functions for meta transactions. Enabling users of this dependencies to carry out transactions on behalf of someone else's wallet, as long as the signer of the transaction can prove he has access to the private key material of the current owner of the identity.

We've added tests for each meta transaction case actually resolving the did document afterwards. (Huge kudos to the whole test suite. It really enabled us to understand all of this fast.)

Some open questions we still have:

• We added a helper function signMetaTxData to sign the data in the correct format for the tests to work. I think this is not normally something this repository really needs thats why we kept it in the dev deps. Is this OK?
• Added an Interface MetaSignature to keep the signed method variants input parameters clean. Should this be something this package also exports for others to use?

[lleifermann]

@mirceanis

Just FYI we are still discussing right now if this repository shouldn't export hashing functions for each of the signed methods. Like hashSetAttributeSigned as this is a very contract specific thing on how this has to be hashed. (This would move the the ethers dev dep to a normal dep though)

TBH i think the best way would be if the contract actually exposed some functions and you let the hashing (before signing) happen via the contract. But as changing the contract is a rather hard way to pull off we think locating this as close as possible to the contract interaction itself might make sense.

This would also enable us in the future that the contract owns this logic. WDYT?

cc @strumswell @dennisvonderbey

[mirceanis]

@mirceanis

Just FYI we are still discussing right now if this repository shouldn't export hashing functions for each of the signed methods. Like hashSetAttributeSigned as this is a very contract specific thing on how this has to be hashed. (This would move the the ethers dev dep to a normal dep though)

TBH i think the best way would be if the contract actually exposed some functions and you let the hashing (before signing) happen via the contract. But as changing the contract is a rather hard way to pull off we think locating this as close as possible to the contract interaction itself might make sense.

This would also enable us in the future that the contract owns this logic. WDYT?

cc @strumswell @DennisVonDerBey

This could also be achieved using a wrapper contract that exposes some pure functions for hashing. But I'm not sure I understand what the difference would be between having the contract do the hashing or performing the hashing off-chain. Is there something else than keccak256 natively available for hashing on EVM ? Or have I misunderstood your ask here?

[lleifermann]

@mirceanis Just FYI we are still discussing right now if this repository shouldn't export hashing functions for each of the signed methods. Like hashSetAttributeSigned as this is a very contract specific thing on how this has to be hashed. (This would move the the ethers dev dep to a normal dep though) TBH i think the best way would be if the contract actually exposed some functions and you let the hashing (before signing) happen via the contract. But as changing the contract is a rather hard way to pull off we think locating this as close as possible to the contract interaction itself might make sense. This would also enable us in the future that the contract owns this logic. WDYT? cc @strumswell @DennisVonDerBey

This could also be achieved using a wrapper contract that exposes some pure functions for hashing. But I'm not sure I understand what the difference would be between having the contract do the hashing or performing the hashing off-chain. Is there something else than keccak256 natively available for hashing on EVM ? Or have I misunderstood your ask here?

This is more of a code style and accessibility question than anything else to be honest. Our reasoning was that code which uses this dependency will then have to exactly do the same procedure of hashing as the contract does. Mirroring the code into upstream dependencies.

It's really a question about 'who should own how params are hashed' and having this as close to the contract interaction as possible made sense to us.

The contract has the requirement that hashing happens in some exact way so it's the direct owner of that requirement. Not having to worry about that in this or other dependencies would've been nice.

[mirceanis]

This is more of a code style and accessibility question than anything else to be honest. Our reasoning was that code which uses this dependency will then have to exactly do the same procedure of hashing as the contract does. Mirroring the code into upstream dependencies.

It's really a question about 'who should own how params are hashed' and having this as close to the contract interaction as possible made sense to us.

I see what you mean. The way I see this, the "signed" methods require a 2 step process, and both steps could be provided by this library.

Step 1, the DID controller calls some createDoSomethingRequest() method, where they are expected to generate the signature Step 2, the request + signature is sent to the transaction signer, which calls the actual methods on the contract, or rather a method from EthrDidController (paying the ethereum transaction fee along the way).

Example:

• DID owner calls createSetAttributeSignerRequest(identity, attrKey, attrValue, rawSigner), which returns an object like: { operation:setAttribute, identity:0x address of the DID, attrKey:<32 byte string>, attrValue:, signature: MetaSignature }
• controller sends this object to the fee payer
• fee payer calls setAttributeSigned() on the contract with the above parameters.

Does this make sense? What do you think?

[lleifermann]

@mirceanis

I've just added changes implementing the things we discussed about. For each meta transaction method we now have a correlating createHash function which generates the correct hash with the needed input parameters. This means that this library really does not have to deal with the key material at all and it fully stays in the callers hands. We just generate the hash and play it back where it then gets signed (for instance in the kms plugin).

Originally we wanted to put this into some abstraction with only one method but that added a lot of complexity. This approach should be rather straight forward to understand and more importantly debug 😄

Not sure exactly on how to test the nonce function against the older version of the contract though. Do you have an example for this with the ganache setup this repo is running with?

[mirceanis]

I've just added changes implementing the things we discussed about. For each meta transaction method we now have a correlating createHash function which generates the correct hash with the needed input parameters. This means that this library really does not have to deal with the key material at all and it fully stays in the callers hands. We just generate the hash and play it back where it then gets signed (for instance in the kms plugin).

I love this approach!

Not sure exactly on how to test the nonce function against the older version of the contract though. Do you have an example for this with the ganache setup this repo is running with?

I don't know either, I haven't done anything like this yet so I don't have an example. The steps to reproduce weird behavior in the old contract:

• create a DID
• changeOwner
• setAttributeSigned (increments nonce for the old owner)
• setAttributeSigned again << fails with bad_signature if nonce is loaded from signer and not from the owner.

Even worse bug in the old contract:

• changeOwnerSigned (increments nonce for the old owner, but hash is computed with nonce from the signer)
• add/removeDelegateSigned (increments nonce for the old owner, but hash is computed with nonce from the signer) This means that owner and delegates can be changed at most once using metaTX, until the owner and signer nonces diverge.

Perhaps @piouslove can jump in with an example, or maybe @nichonien has run into this issue at EW

[lleifermann]

@mirceanis Now everything should be in-place i believe 😄 - For the tests i had to copy the ABI of the old version of the contract as old packages don't export the abi sadly. But the two new tests shoud cover the compatability of nonces.

[uport-automation-bot]

:tada: This PR is included in version 6.2.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: