Harbour-MVP icon indicating copy to clipboard operation
Harbour-MVP copied to clipboard

Format of signature

Open s1na opened this issue 5 years ago • 6 comments

One of the differences I have seen among projects is how they transmit the signature of the meta tx, with some sending v, r, s fields separately, and some using an encoded version.

What are the arguments for each? For an encoded signature, which EIP does it adhere to (EIP-191 or EIP-712)?

s1na avatar Sep 20 '18 08:09 s1na

This is a really great place to start! I've seen it done both ways and I would love to hear the pros and cons of each. I used the encoded version because that's what came with the ECDSA contracts from OpenZeppelin. To split the bytes they do:

    assembly {
      r := mload(add(signature, 32))
      s := mload(add(signature, 64))
      v := byte(0, mload(add(signature, 96)))
    }

The gas cost of this operation is minimal but I think that is the argument for splitting off-chain. However, keeping it together as bytes means you have more room for other arguments and variables on the stack. I don't know about you guys but "Stack Too Deep" errors are my worst enemy.

austintgriffith avatar Sep 20 '18 14:09 austintgriffith

Yeah, we were passing the V, R, and S separately for the assumed increased gas costs. (We hadn't tested these assumptions to see the true cost of doing this operation on chain). Have not run into the "Stack Too Deep" error, but I would agree that if the cost is negligible then the standard should follow that we do the operation on chain for cleaner smart contract code readability and to Austin's point, to allow for more arguments.

Kyrrui avatar Sep 20 '18 14:09 Kyrrui

Same here, I just assumed it will cost more gas, but I haven't tested the exact amount. My line of thought is that even a small saving on gas can make a huge difference if this is going to happen on every transaction. With regards to "Stack Too Deep" I've hit it sometimes, but normally walk around it by shuffling local variables.

radek1st avatar Sep 21 '18 01:09 radek1st

I definitely agree with the gas concern. On the other hand, going through EIP-191, these concerns have been raised for sending signature parts separetely (in multisig wallets):

  • An Ethereum TX that was signed by the user, which has the format RLP<nonce, gasPrice, startGas, to, value, data>, r, s, v, can be republished to the relay network and submitted to the network. I'm not sure if this would likely cause an issue (if the implementation doesn't use the same encoding, and does validation), due to nonce mismatch.
  • If multiple projects adopt the same standard, their meta-txes couldn't be distinguished from one another. I think then It'd be possible to submit a meta tx the user makes in one wallet, to another one.

And, because meta txes are structured, adopting EIP-712, which kind of supersedes EIP-191 and seems to be finding consensus among developers, would help with usability and security. Metamask is adding support soon, and the Gnosis team also mentioned, they will probably make use of it for Safe.

s1na avatar Sep 23 '18 17:09 s1na

Previous chat logs from the telegram group (archival purposes) https://hackmd.io/NPzA_EEFTuWR_DnnEEHOyw

pet3r-pan avatar Sep 24 '18 14:09 pet3r-pan

If you have 1 signature it should be more gas efficient to pass the parts in separately (since bytes would be a dynamic type that is automatically copied to memory and therfore has some overhead). You can actually optimize this by hacking around a little in solitidy (external functions don't automatically copy their types into memory if not used).

The thing is that I would suggest to support multiple signatures and also standards like EIP-1271. For this it is way more dynamic to use a bytes

rmeissner avatar Oct 18 '18 08:10 rmeissner