-
Notifications
You must be signed in to change notification settings - Fork 31
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
[Draft only] Specification v2 #28
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite there yet, but on the right track. I was expecting to see more content about some of the new design details.
- 2^24 accounts
- arbitrarily many orders
- something about subset proofs.
- etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some questions inline.
- verify that cancelation happend after order placement | ||
- verify that cancelation is valid by reconstructing rolling order hash | ||
|
||
Order already settled in previous auction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we going to handle partial settlement? Is an order considered settled once it is touched with an epsilon? Or do we have to compute the sum of all previous settlements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is an order considered settled once it is touched with an epsilon?
Yes.
It is a shitty user experience, but for simplicity, I guess it is the best for now.
dFusion/dFusion.rst
Outdated
----------------- | ||
- provide unique reference to canceled order in solution | ||
- provide unique reference to cancelation: provide index and stored rolling order hash for cancelation | ||
- verify that cancelation happend after order placement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think we also have to verify that cancellation happened before solution finding started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for highlighting this point.
-> actually, we just check that the cancelation was within the order stream of the last 2**24 orders counted from the auction closing.
However, the order cancelation could also happen before the actual order, and still, we want cancelation to be valid (as orders could be replayed).
- Check that **tradingSurplus == input.tradingSurplus** | ||
- For all balances, check that **balance > 0** | ||
- return **newstate** by hashing all balances together (with pedersen) | ||
Fraud-Proofs for auction settlements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be useful to state the amount of data needed for each challenge?
E.g. for order cancellation one would need |solution| + |order batch| + |cancellation batch|
whereas for Order already settled in previous auction one would need 2 * |solution|
Hey, sorry, I didn't realize that I had reviewed on a single commit. I will go through all the changes again! |
Co-Authored-By: Benjamin Smith <[email protected]>
|
||
Along with the orders, the solution submitter also has to post a vector **V** of **buyVolumes** and **sellVolumes** for each order: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should be using Amounts
here rather than Volumes
. This is due to the fact that one of them is in reference to the unit amount of execution where the other (volume) is with respect to price. volume = amount * price
. @twalth3r might want to add his explanation here as well.
|
||
|
||
Anyone can caluclate the **sellVolume** from the price of the token pair and the buyVolume. | ||
Theoretically, it would be sufficient to provide only the **sellVolumes** and then caculate the **buyVolumes** from the prices. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting point! 😎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except practically we will have a slight loss of precision...
|
||
|
||
Anyone can caluclate the **sellVolume** from the price of the token pair and the buyVolume. | ||
Theoretically, it would be sufficient to provide only the **sellVolumes** and then caculate the **buyVolumes** from the prices. | ||
However, then rounding errors could happen, which will also effect the constraints of the optimization problem in unforseen ways and finding a solution generally becomes harder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, then rounding errors could happen, which will also effect the constraints of the optimization problem in unforseen ways and finding a solution generally becomes harder. | |
However, then rounding errors could happen, which could affect the constraints of the optimization problem in unforeseen ways and finding a solution generally becomes harder. |
The number of intermediate hashes depends on the amount of orders in a solution. | ||
The exact number will be optimized at a later state, but roughly the number will be around _amount_of_orders_ / 5 . | ||
|
||
The new state is optimistically assumed correct and only fraud proofs can invalidate them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new state is optimistically assumed correct and only fraud proofs can invalidate them. | |
Updated state are optimistically assumed to be correct and can only be invalidated by fraud proofs. |
- Reprovide the solution as payload | ||
- For each order processed, verify the order from orderstream by reconstructing the stored order rolling hash of the referenced batch | ||
- Verify hashed volume calldata matches committed volume hash of solution | ||
- For each volume: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should be a beginning of a nested list...
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
------- | ||
- verify via a merkle proof that the last intermediate-state-root does not hold the claimed utility | ||
|
||
Conservation of value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description of this seems somewhat understated. As the code is currently written, the conservation of value proof is for a specific token.
@@ -376,35 +434,25 @@ The deposits can be incorporated by any significantly bonded party by calling th | |||
|
|||
This function would update the **state** by incorporating the deposits received from **blockNr** to **blockNr+19**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to be relevant any longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be completely honest, I found a lot of careless mistakes that could have been avoided with spell check. I think that the amount of changes made here could have been broken down into several smaller tasks and submitted as multiple PRs. In particular, I would like to see the Fraud-proof section taken out of this PR, written more carefully and submitted as a stand-alone PR.
I have already spent a substantial amount of time on this making small corrections I think it could have been much less if the changes were submitted in much smaller chunks.
It would be nice to see a few more links to code chunks that are directly related to the step-by-step. process described in the on-chain verification.
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
Co-Authored-By: Benjamin Smith <[email protected]>
We have several new and good ideas spread over several issues.
This PR tries to consolidate them into one specfication_v2
It contains all new ideas regarding:
Missing is certainly a profound gas analysis and any fee considerations.