elixir-omg
elixir-omg copied to clipboard
More flexible struct-hash implementation
This is a set of ideas that have emerged in #827, omisego/plasma-contracts#189 discussions. I'm compiling some of the comments to this issue.
Rationale
Abstract Layer Design has already changed and extended Transaction format. Also we gained some experience from initial EIP-712 implementation.
- Transaction doesn't require padding in/outputs with placeholders/empty values
- There can be more in/outputs than 4 and also less (e.g. deposit with no inputs and single output)
- Make the metadata optional again (and do it right)
- ...
To align with above we can leverage latest Metamask improvements (original message from @kevsul):
Support for dynamic arrays has just been added to the eth-sig-util package (🎉), which is what Metamask uses. The current Metamask (7.0.1) version doesn't support it, but it's been merged to master so next version will.
https://github.com/MetaMask/eth-sig-util/pull/54
This will allow us to specify transaction metadata as byte[]
instead of byte32
. The metadata field is still mandatory, but it can be empty, which I think is a better solution than having 2 different transaction types for with and without metadata.
Note also that this allows us to get rid of the input and output 'padding' that we currently do, and change the tx spec to something like this:
const txSpec = [
{ name: 'txType', type: 'uint256' },
{ name: 'inputs', type: 'Input[]' },
{ name: 'outputs', type: 'Output[]' },
{ name: 'metadata', type: 'byte[]' }
]
This won't work for current elixir-omg implementation because the contract code makes assumptions that the tx itself always has 4 inputs and 4 outputs, but I think we can use it in abstract layer.
If we require that the transaction itself has to have all of the above members, then we can avoid the 2 versions of the 'same' transaction with different identifiers as described in this issue.
This is tricky because unless we have a separate type for deposit tx (and this type is encoded in txbytes) this is indistinguishable from any other tx (blknum is not part of txbytes)
A deposit tx then is a tx with an empty inputs
array, so it will be distinguishable.
It looks like Metamask supports this now. Worth further investigation
Is this still relevant @pnowosie @pdobacz ? If yes, please re-open!
getting rid of the padding in EIP712 would be great, maintaining those padders is going to be a burden. Still relevant, reopening