Skip to content
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

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

[Draft only] Specification v2 #28

wants to merge 29 commits into from

Conversation

josojo
Copy link
Contributor

@josojo josojo commented Jul 19, 2019

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:

  • order-streams
  • order cancelations
  • order-subset considerations
  • multi-solution submission
  • using on-chain verification
  • overview of all fraud-proofs

Missing is certainly a profound gas analysis and any fee considerations.

@josojo josojo requested review from fleupold, bh2smith and twalth3r July 19, 2019 09:33
Copy link
Contributor

@bh2smith bh2smith left a 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...

Copy link
Contributor

@fleupold fleupold left a 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
Copy link
Contributor

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?

Copy link
Contributor Author

@josojo josojo Jul 22, 2019

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.

-----------------
- 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
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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|

@bh2smith
Copy link
Contributor

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:
Copy link
Contributor

@bh2smith bh2smith Jul 22, 2019

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting point! 😎

Copy link
Contributor

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.
Copy link
Contributor

@bh2smith bh2smith Jul 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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:
Copy link
Contributor

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...

josojo and others added 2 commits July 22, 2019 13:57
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
Copy link
Contributor

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**.
Copy link
Contributor

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.

@bh2smith bh2smith self-requested a review July 22, 2019 12:13
Copy link
Contributor

@bh2smith bh2smith left a 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.

josojo and others added 8 commits July 27, 2019 16:15
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants