AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors #1294

Chralt98 · 2024-03-29T10:20:21Z

What does it do?

What important points should reviewers know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues?

References

Chralt98 · 2024-04-02T14:24:43Z

zrml/neo-swaps/src/lib.rs

@@ -1005,6 +1005,27 @@ mod pallet {
                external_fee_percentage.mul_floor(ZeitgeistBase::<BalanceOf<T>>::get()?);
            swap_fee.checked_add_res(&external_fee_fractional)
        }
+
+        fn match_failure(error: DispatchError) -> ApiError<AmmSoftFail> {
+            // TODO what else should be a soft failure for the Hybrid Router?


What other errors should lead to soft failures for order book and AMM?

codecov-commenter · 2024-04-03T09:47:27Z

Codecov Report

Attention: Patch coverage is 57.31707% with 35 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (chralt98-amm-cda-release@1fb8a4b). Click here to learn what that means.

Files	Patch %	Lines
zrml/neo-swaps/src/lib.rs	20.00%	16 Missing ⚠️
zrml/orderbook/src/lib.rs	28.57%	10 Missing ⚠️
zrml/hybrid-router/src/lib.rs	81.25%	9 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@                     Coverage Diff                     @@
##             chralt98-amm-cda-release    #1294   +/-   ##
===========================================================
  Coverage                            ?   92.27%           
===========================================================
  Files                               ?      165           
  Lines                               ?    33551           
  Branches                            ?        0           
===========================================================
  Hits                                ?    30958           
  Misses                              ?     2593           
  Partials                            ?        0

Flag	Coverage Δ
tests	`92.27% <57.31%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mergify · 2024-04-04T08:11:44Z

This pull request is now in conflicts. Could you fix it @Chralt98? 🙏

zrml/hybrid-router/Cargo.toml

zrml/neo-swaps/src/lib.rs

zrml/orderbook/src/lib.rs

zrml/neo-swaps/src/lib.rs

* AMM-CDA-1: Switch to AmmCdaHybrid enum field (#1274) * switch to AmmCdaHybrid enum field * Update zrml/parimutuel/src/tests/buy.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update zrml/parimutuel/src/tests/claim.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update zrml/parimutuel/src/tests/refund.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update storage version to 11 and add MigrateScoringRuleAmmCdaHybrid * Update primitives/src/market.rs * update migration --------- Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * merge amm-cda-3 changes * merge amm-cda-2 changes * merge amm-cda-4 changes * correct clippy * merge amm-cda-5 changes * merge amm-cda-6 changes * Fix Hybrid Router clippy and tests (#1291) * glue everything together * add hybrid router to configuration * fix tests * fix conditional tests * AMM-CDA-8 Add event information for fees and aggregated amount_out (#1293) * add event info * use slice as function parameter * update test amm amount out of event * fix order book tests * update documentation * removed dependency * AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors (#1294) * add event info * use slice as function parameter * update test amm amount out of event * wip * handle soft and hard failure * add order book soft failure * fix clippy * fix CI * remove swaps pallet dependency * add compact to order book struct * fix recursion overflow * Fix of Hybrid Router after asset system merge (#1309) * wip * use asset conversions * adapt hybrid router to new asset system * apply review suggestions * fmt * rename Asset to Assets * update copyrights * update hybrid router crate version * correct orderbook spelling * add amount is zero tests * add price limit too high test * add max order exceeded test * use saturated conversion * use saturated conversion again * remove unused code * add tests for soft failures * add skip order test * add changelog for devs description * add amm soft failure test * add numerical soft failure test for sell * correct failing test for parachain feature * cover remaining is zero execution path --------- Co-authored-by: Malte Kliemann <mail@maltekliemann.com>

* Add market ID to `Market` struct (#1248) * Add market ID to `Market` struct * Add market builder struct * Use `PredictionMarketBuilder` * Fix issues * Fix copyright * Make `build` return a `Result` * Let `build` raise an error on incomplete data. * Fix formatting * Refactor `MarketBuilder` * Add missing files * AMM-CDA-1: Switch to AmmCdaHybrid enum field (#1274) * switch to AmmCdaHybrid enum field * Update zrml/parimutuel/src/tests/buy.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update zrml/parimutuel/src/tests/claim.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update zrml/parimutuel/src/tests/refund.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update storage version to 11 and add MigrateScoringRuleAmmCdaHybrid * Update primitives/src/market.rs * update migration --------- Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * merge amm-cda-3 changes * merge amm-cda-2 changes * merge amm-cda-4 changes * correct clippy * merge amm-cda-5 changes * merge amm-cda-6 changes * Fix Hybrid Router clippy and tests (#1291) * glue everything together * add hybrid router to configuration * fix tests * fix conditional tests * AMM-CDA-8 Add event information for fees and aggregated amount_out (#1293) * add event info * use slice as function parameter * update test amm amount out of event * fix order book tests * update documentation * removed dependency * Merge `main` into `mkl-market-id-feature` (#1302) * Merge `main` into `mkl-mkl-market-id-feature` * Remove unused `outcomes` * Remove old migrations (#1301) * AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors (#1294) * add event info * use slice as function parameter * update test amm amount out of event * wip * handle soft and hard failure * add order book soft failure * fix clippy * fix CI * remove swaps pallet dependency * add compact to order book struct * fix recursion overflow * Migration: Add Market ID to Market (#1257) * Add market ID to `Market` struct * Add market builder struct * Use `PredictionMarketBuilder` * Fix issues * Fix copyright * Make `build` return a `Result` * Let `build` raise an error on incomplete data. * Fix formatting * Refactor `MarketBuilder` * Add missing files * Add migration to new market * Fix migration * Fix missing import * Fix duplicate import * Fix formatting * Remove unused `types/` * Fix of Hybrid Router after asset system merge (#1309) * wip * use asset conversions * adapt hybrid router to new asset system * apply review suggestions * fmt * rename Asset to Assets * update copyrights * Make minor fixes after merge * update hybrid router crate version * correct orderbook spelling * add amount is zero tests * add price limit too high test * add max order exceeded test * use saturated conversion * use saturated conversion again * remove unused code * Update changelog * add tests for soft failures * add skip order test * add changelog for devs description * add amm soft failure test * add numerical soft failure test for sell * Remove ZeitgeistAssetManager trait * correct failing test for parachain feature * cover remaining is zero execution path * Fix try-runtime test --------- Co-authored-by: Chralt <chralt.developer@gmail.com> Co-authored-by: Chralt98 <chralt98@gmail.com> Co-authored-by: Harald Heckmann <mail@haraldheckmann.de>

Chralt98 added 4 commits March 29, 2024 08:03

add event info

164a70c

use slice as function parameter

68db417

update test amm amount out of event

0964acc

wip

07a5bce

Chralt98 added the s:in-progress The pull requests is currently being worked on label Mar 29, 2024

Chralt98 self-assigned this Mar 29, 2024

handle soft and hard failure

50db51d

Chralt98 commented Apr 2, 2024

View reviewed changes

Chralt98 added 2 commits April 3, 2024 11:21

add order book soft failure

43ba489

fix clippy

e5061b9

Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Apr 3, 2024

Chralt98 marked this pull request as ready for review April 3, 2024 09:38

Chralt98 requested a review from maltekliemann as a code owner April 3, 2024 09:38

Base automatically changed from chralt98-amm-cda-8 to chralt98-amm-cda-release April 4, 2024 08:11

mergify bot added s:revision-needed The pull requests must be revised and removed s:review-needed The pull request requires reviews labels Apr 4, 2024

Merge branch 'chralt98-amm-cda-release' into chralt98-amm-cda-9

a3d50bd

mergify bot added s:in-progress The pull requests is currently being worked on and removed s:revision-needed The pull requests must be revised labels Apr 4, 2024

Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Apr 4, 2024

fix CI

a148ec7

mergify bot added s:in-progress The pull requests is currently being worked on and removed s:review-needed The pull request requires reviews labels Apr 4, 2024

Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Apr 4, 2024

maltekliemann approved these changes Apr 4, 2024

View reviewed changes

zrml/hybrid-router/Cargo.toml Outdated Show resolved Hide resolved

zrml/neo-swaps/src/lib.rs Show resolved Hide resolved

zrml/orderbook/src/lib.rs Show resolved Hide resolved

remove swaps pallet dependency

e89f248

mergify bot added s:in-progress The pull requests is currently being worked on and removed s:review-needed The pull request requires reviews labels Apr 8, 2024

Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Apr 8, 2024

maltekliemann approved these changes Apr 8, 2024

View reviewed changes

zrml/neo-swaps/src/lib.rs Show resolved Hide resolved

Chralt98 merged commit a7f61b7 into chralt98-amm-cda-release Apr 8, 2024
18 of 19 checks passed

Chralt98 deleted the chralt98-amm-cda-9 branch April 8, 2024 12:56

Chralt98 mentioned this pull request Apr 8, 2024

Implement Hybrid Router #1307

Merged

Chralt98 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors #1294

AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors #1294

Chralt98 commented Mar 29, 2024

Chralt98 Apr 2, 2024

codecov-commenter commented Apr 3, 2024 •

edited

Loading

mergify bot commented Apr 4, 2024

AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors #1294

AMM-CDA-9 Adds soft and hard failure distinction for AMM and order book errors #1294

Conversation

Chralt98 commented Mar 29, 2024

What does it do?

What important points should reviewers know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues?

References

Chralt98 Apr 2, 2024

Choose a reason for hiding this comment

codecov-commenter commented Apr 3, 2024 • edited Loading

Codecov Report

mergify bot commented Apr 4, 2024

codecov-commenter commented Apr 3, 2024 •

edited

Loading