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

feat: implement batch limit order at router #47

Merged
merged 6 commits into from
May 15, 2023
Merged

feat: implement batch limit order at router #47

merged 6 commits into from
May 15, 2023

Conversation

JhChoy
Copy link
Contributor

@JhChoy JhChoy commented May 12, 2023

  • Implement batch limit order logic at Router.
  • Change refund logic to support batch order

@github-actions

This comment has been minimized.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: -11.24 ⚠️

Comparison is base (be7147b) 98.81% compared to head (2db3a97) 87.57%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main      #47       +/-   ##
===========================================
- Coverage   98.81%   87.57%   -11.24%     
===========================================
  Files           8       10        +2     
  Lines         589      684       +95     
  Branches      108      146       +38     
===========================================
+ Hits          582      599       +17     
- Misses          2       80       +78     
  Partials        5        5               
Impacted Files Coverage Δ
contracts/ArithmeticPriceBook.sol 100.00% <ø> (ø)
contracts/GeometricPriceBook.sol 0.00% <ø> (ø)
contracts/MarketDeployer.sol 100.00% <ø> (ø)
contracts/OrderNFTDeployer.sol 100.00% <ø> (ø)
contracts/PriceBookDeployer.sol 100.00% <ø> (ø)
contracts/MarketFactory.sol 97.46% <100.00%> (ø)
contracts/MarketRouter.sol 97.95% <100.00%> (+0.39%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@graykode graykode left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

Test with e467575.

  • EHET : Empty heap with empty segment tree
  • EHDT : Empty heap with dirty segment tree
  • FHET : Filled heap with empty segment tree
  • FHFT : Filled heap with filled segment tree

Changes to gas cost

Generated at commit: e467575fb209ef6544a5803f462237be60ada1da, compared to commit: d7aec75815e6abaf66024b0bfece9bb3851019ed

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
GasReporter contract EHDT_LimitAsk
EHET_LimitAsk
FHET_LimitAsk
FHFT_LimitAsk
limitAskOrder
limitBidOrder
+152 ❌
+152 ❌
+152 ❌
+152 ❌
+113 ❌
+112 ❌
+0.43%
+0.43%
+0.39%
+0.39%
+0.15%
+0.13%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
GasReporter contract 948,376 (0) EHDT_FullyClaimAsk
EHDT_FullyClaimBid
EHDT_FullyMarketAsk
EHDT_FullyMarketBid
EHDT_LimitAsk
EHDT_LimitBid
EHDT_PartiallyClaimAsk
EHDT_PartiallyClaimBid
EHDT_PartiallyMarketAsk
EHDT_PartiallyMarketBid
EHET_FullyClaimAsk
EHET_FullyClaimBid
EHET_FullyMarketAsk
EHET_FullyMarketBid
EHET_LimitAsk
EHET_LimitBid
EHET_PartiallyClaimAsk
EHET_PartiallyClaimBid
EHET_PartiallyMarketAsk
EHET_PartiallyMarketBid
FHET_FullyClaimAsk
FHET_FullyClaimBid
FHET_FullyMarketAsk
FHET_FullyMarketBid
FHET_LimitAsk
FHET_LimitBid
FHET_PartiallyClaimAsk
FHET_PartiallyClaimBid
FHET_PartiallyMarketAsk
FHET_PartiallyMarketBid
FHFT_FullyClaimAsk
FHFT_FullyClaimBid
FHFT_FullyMarketAsk
FHFT_FullyMarketBid
FHFT_LimitAsk
FHFT_LimitBid
FHFT_PartiallyClaimAsk
FHFT_PartiallyClaimBid
FHFT_PartiallyMarketAsk
FHFT_PartiallyMarketBid
limitAskOrder
limitBidOrder
marketAskOrder
marketBidOrder
84,371 (+15)
84,298 (+15)
117,321 (+81)
120,890 (+57)
35,469 (+152)
162,311 (+117)
68,624 (+15)
68,583 (+15)
117,343 (+81)
113,477 (+57)
84,350 (+15)
84,297 (+15)
117,408 (+81)
105,032 (+45)
35,493 (+152)
245,173 (+117)
68,580 (+15)
68,538 (+15)
117,342 (+81)
96,379 (+57)
33,504 (+15)
33,482 (+15)
117,409 (+81)
105,066 (+46)
38,897 (+152)
171,755 (+117)
33,404 (+15)
33,405 (+15)
117,342 (+81)
96,379 (+57)
33,503 (+15)
33,483 (+15)
117,365 (+81)
120,912 (+57)
38,982 (+152)
122,990 (+117)
33,362 (+15)
33,381 (+15)
117,343 (+81)
113,499 (+57)
49,467 (+94)
45,570 (+94)
87,454 (+81)
46,454 (+46)
+0.02%
+0.02%
+0.07%
+0.05%
+0.43%
+0.07%
+0.02%
+0.02%
+0.07%
+0.05%
+0.02%
+0.02%
+0.07%
+0.04%
+0.43%
+0.05%
+0.02%
+0.02%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.04%
+0.39%
+0.07%
+0.04%
+0.04%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.05%
+0.39%
+0.10%
+0.04%
+0.04%
+0.07%
+0.05%
+0.19%
+0.21%
+0.09%
+0.10%
84,371 (+15)
84,298 (+15)
117,321 (+81)
120,890 (+57)
35,469 (+152)
162,311 (+117)
68,624 (+15)
68,583 (+15)
117,343 (+81)
113,477 (+57)
84,350 (+15)
84,297 (+15)
117,408 (+81)
105,032 (+45)
35,493 (+152)
245,173 (+117)
68,580 (+15)
68,538 (+15)
117,342 (+81)
96,379 (+57)
33,504 (+15)
33,482 (+15)
117,409 (+81)
105,066 (+46)
38,897 (+152)
171,755 (+117)
33,404 (+15)
33,405 (+15)
117,342 (+81)
96,379 (+57)
33,503 (+15)
33,483 (+15)
117,365 (+81)
120,912 (+57)
38,982 (+152)
122,990 (+117)
33,362 (+15)
33,381 (+15)
117,343 (+81)
113,499 (+57)
77,592 (+113)
85,443 (+112)
88,454 (+81)
51,260 (+51)
+0.02%
+0.02%
+0.07%
+0.05%
+0.43%
+0.07%
+0.02%
+0.02%
+0.07%
+0.05%
+0.02%
+0.02%
+0.07%
+0.04%
+0.43%
+0.05%
+0.02%
+0.02%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.04%
+0.39%
+0.07%
+0.04%
+0.04%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.05%
+0.39%
+0.10%
+0.04%
+0.04%
+0.07%
+0.05%
+0.15%
+0.13%
+0.09%
+0.10%
84,371 (+15)
84,298 (+15)
117,321 (+81)
120,890 (+57)
35,469 (+152)
162,311 (+117)
68,624 (+15)
68,583 (+15)
117,343 (+81)
113,477 (+57)
84,350 (+15)
84,297 (+15)
117,408 (+81)
105,032 (+45)
35,493 (+152)
245,173 (+117)
68,580 (+15)
68,538 (+15)
117,342 (+81)
96,379 (+57)
33,504 (+15)
33,482 (+15)
117,409 (+81)
105,066 (+46)
38,897 (+152)
171,755 (+117)
33,404 (+15)
33,405 (+15)
117,342 (+81)
96,379 (+57)
33,503 (+15)
33,483 (+15)
117,365 (+81)
120,912 (+57)
38,982 (+152)
122,990 (+117)
33,362 (+15)
33,381 (+15)
117,343 (+81)
113,499 (+57)
82,865 (+117)
97,894 (+117)
88,454 (+81)
51,260 (+51)
+0.02%
+0.02%
+0.07%
+0.05%
+0.43%
+0.07%
+0.02%
+0.02%
+0.07%
+0.05%
+0.02%
+0.02%
+0.07%
+0.04%
+0.43%
+0.05%
+0.02%
+0.02%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.04%
+0.39%
+0.07%
+0.04%
+0.04%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.05%
+0.39%
+0.10%
+0.04%
+0.04%
+0.07%
+0.05%
+0.14%
+0.12%
+0.09%
+0.10%
84,371 (+15)
84,298 (+15)
117,321 (+81)
120,890 (+57)
35,469 (+152)
162,311 (+117)
68,624 (+15)
68,583 (+15)
117,343 (+81)
113,477 (+57)
84,350 (+15)
84,297 (+15)
117,408 (+81)
105,032 (+45)
35,493 (+152)
245,173 (+117)
68,580 (+15)
68,538 (+15)
117,342 (+81)
96,379 (+57)
33,504 (+15)
33,482 (+15)
117,409 (+81)
105,066 (+46)
38,897 (+152)
171,755 (+117)
33,404 (+15)
33,405 (+15)
117,342 (+81)
96,379 (+57)
33,503 (+15)
33,483 (+15)
117,365 (+81)
120,912 (+57)
38,982 (+152)
122,990 (+117)
33,362 (+15)
33,381 (+15)
117,343 (+81)
113,499 (+57)
218,265 (+117)
237,304 (+117)
89,454 (+81)
56,067 (+57)
+0.02%
+0.02%
+0.07%
+0.05%
+0.43%
+0.07%
+0.02%
+0.02%
+0.07%
+0.05%
+0.02%
+0.02%
+0.07%
+0.04%
+0.43%
+0.05%
+0.02%
+0.02%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.04%
+0.39%
+0.07%
+0.04%
+0.04%
+0.07%
+0.06%
+0.04%
+0.04%
+0.07%
+0.05%
+0.39%
+0.10%
+0.04%
+0.04%
+0.07%
+0.05%
+0.05%
+0.05%
+0.09%
+0.10%
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
245,784 (0)
245,784 (0)
4 (0)
4 (0)

@JhChoy JhChoy merged commit 1bcaf60 into main May 15, 2023
10 checks passed
@JhChoy JhChoy deleted the feat/router branch May 15, 2023 02:20
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.

None yet

4 participants