Add order commitment check on verification #21
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to have at most a single valid AMM order on any batch while at the same time providing the flexibility to solvers to create arbitrary orders within the constraints of the AMM function.
This PR enforces this by a commitment mechanism.
If no commitment is set, the verification function assumes that there is an implicit commitment to the order obtained from
getTradeableOrder
and it checks that the input order parameters match.Any other previously valid order
o
can be settled in a batch as follows.commit(ammAddress, o.hash())
in a pre-interaction.commit(ammAddress, EMPTY_COMMITMENT)
in a post-interaction to unset the commitment and get back some gas as refund.It would be really nice to use
TSTORE
to avoid point 3 and to make this process much cheaper but we're a month too early.Drawbacks of these changes: calling
getTradeableOrder
or committing means an increased gas cost when settling the order.To do (in another PR)
Update the docs to explain the role of commitments and their intended usage.
How to test
Existing and new unit tests. Here is a test order for this code.
You can get a feeling for the gas cost by resimulating without access list. The simulation highlights that the cost of the extra call to perform the extra checks is about 31k gas.
I also tested that, assuming a commit is set and an order is possible, the watchtower doesn't stop indexing the order. (See internal logs here.) It does become noisy however.