This repository was archived by the owner on Apr 28, 2022. It is now read-only.
Pre-Order Validation #1169
Merged
Pre-Order Validation #1169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bh2smith
force-pushed
the
orderbook-interface
branch
from
af96b09 to
53bd145
Compare
Codecov Report
@@ Coverage Diff @@
## main #1169 +/- ##
==========================================
+ Coverage 66.63% 66.74% +0.11%
==========================================
Files 134 135 +1
Lines 26843 27025 +182
==========================================
+ Hits 17886 18039 +153
- Misses 8957 8986 +29 |
2 tasks
vkgnosis
approved these changes
Sep 29, 2021
orderbook/src/api/validation.rs
Outdated
Comment on lines
69
to
75
| owner: H160, | ||
| sell_token: H160, | ||
| buy_token: H160, | ||
| receiver: H160, | ||
| valid_to: u32, | ||
| buy_token_balance: BuyTokenDestination, | ||
| sell_token_balance: SellTokenSource, |
There was a problem hiding this comment.
Probably want all of these pub too.
There was a problem hiding this comment.
Yea good call, I made these all public in the next PR, but I can do it here in favour of less changes in the next one.
nlordell
reviewed
Sep 30, 2021
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn validate_partial_order_err() { |
There was a problem hiding this comment.
One underrated feature of this refactor is the ability to test order validation better.
There was a problem hiding this comment.
Perhaps in a follow up PR, I can make a similar method for the other parts of order validation.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
In Preparation for the new Fee and Quote endpoint (cf #1155) we shift some of the order validation into its own file (to be shared among both
Orderbook.add_orderand theQuoteRequestendpoint). More specifically, we move all order validations pertaining to the buy/sell tokens, order validity, balance sources and destinations, banned users, and transfer eth to contract into their ownPreOrderValidator. Any order validations regarding balance or fee sufficiency and signature validity remain in theOrderbook.add_ordermethod.Test Plan
This is a refactor although I have added some tests to the new method
validate_partial_order