AMM-CDA-8 Add event information for fees and aggregated amount_out #1293

Chralt98 · 2024-03-29T07:05:25Z

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-03-29T07:30:52Z

CI failed for a test. I fix it quickly.

codecov-commenter · 2024-03-29T10:29:53Z

Codecov Report

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

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

Files	Patch %	Lines
zrml/hybrid-router/src/lib.rs	79.77%	18 Missing ⚠️
zrml/hybrid-router/src/types.rs	82.14%	5 Missing ⚠️
primitives/src/hybrid_router_api_types.rs	0.00%	3 Missing ⚠️
zrml/orderbook/src/lib.rs	94.44%	1 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    #1293   +/-   ##
===========================================================
  Coverage                            ?   92.36%           
===========================================================
  Files                               ?      165           
  Lines                               ?    33500           
  Branches                            ?        0           
===========================================================
  Hits                                ?    30942           
  Misses                              ?     2558           
  Partials                            ?        0

Flag	Coverage Δ
tests	`92.36% <88.75%> (?)`

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.

maltekliemann · 2024-03-29T12:00:35Z

primitives/src/traits/hybrid_router_amm_api.rs

@@ -89,7 +93,7 @@ pub trait HybridRouterAmmApi {
        asset_out: Self::Asset,


The documentation is outdated now.

maltekliemann · 2024-03-29T12:00:43Z

primitives/src/traits/hybrid_router_amm_api.rs

@@ -129,5 +133,5 @@ pub trait HybridRouterAmmApi {
        asset_in: Self::Asset,
        amount_in: Self::Balance,


The documentation is outdated now.

maltekliemann · 2024-03-29T12:01:27Z

zrml/hybrid-router/Cargo.toml

@@ -30,6 +30,7 @@ zrml-neo-swaps = { workspace = true, optional = true }
 zrml-orderbook = { workspace = true, optional = true }
 zrml-prediction-markets = { workspace = true, optional = true }
 zrml-simple-disputes = { workspace = true, optional = true }
+zrml-swaps = { workspace = true, optional = true }


This should be removed.

* 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 2 commits March 29, 2024 08:03

add event info

164a70c

use slice as function parameter

68db417

Chralt98 added the s:review-needed The pull request requires reviews label Mar 29, 2024

Chralt98 self-assigned this Mar 29, 2024

Chralt98 requested a review from maltekliemann as a code owner March 29, 2024 07:05

Chralt98 mentioned this pull request Mar 29, 2024

AMM-CDA-7 Add event information for indexer for Hybrid Orderbook AMM #1288

Closed

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

update test amm amount out of event

0964acc

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

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 Mar 29, 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 Mar 29, 2024

fix order book tests

14b3819

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 Mar 29, 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 Mar 29, 2024

maltekliemann reviewed Apr 2, 2024

View reviewed changes

update documentation

ad0ec1c

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 2, 2024

removed dependency

16669c7

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 2, 2024

Chralt98 requested a review from maltekliemann April 2, 2024 14:39

maltekliemann approved these changes Apr 3, 2024

View reviewed changes

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

Chralt98 merged commit 1fb8a4b into chralt98-amm-cda-release Apr 4, 2024
19 of 21 checks passed

Chralt98 deleted the chralt98-amm-cda-8 branch April 4, 2024 08:11

Chralt98 mentioned this pull request Apr 8, 2024

Implement Hybrid Router #1307

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AMM-CDA-8 Add event information for fees and aggregated amount_out #1293

AMM-CDA-8 Add event information for fees and aggregated amount_out #1293

Chralt98 commented Mar 29, 2024

Chralt98 commented Mar 29, 2024

codecov-commenter commented Mar 29, 2024

maltekliemann Mar 29, 2024

Chralt98 Apr 2, 2024

maltekliemann Mar 29, 2024

Chralt98 Apr 2, 2024

maltekliemann Mar 29, 2024

Chralt98 Apr 2, 2024

		@@ -89,7 +93,7 @@ pub trait HybridRouterAmmApi {
		asset_out: Self::Asset,

		@@ -129,5 +133,5 @@ pub trait HybridRouterAmmApi {
		asset_in: Self::Asset,
		amount_in: Self::Balance,

AMM-CDA-8 Add event information for fees and aggregated amount_out #1293

AMM-CDA-8 Add event information for fees and aggregated amount_out #1293

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 commented Mar 29, 2024

codecov-commenter commented Mar 29, 2024

Codecov Report

maltekliemann Mar 29, 2024

Choose a reason for hiding this comment

Chralt98 Apr 2, 2024

Choose a reason for hiding this comment

maltekliemann Mar 29, 2024

Choose a reason for hiding this comment

Chralt98 Apr 2, 2024

Choose a reason for hiding this comment

maltekliemann Mar 29, 2024

Choose a reason for hiding this comment

Chralt98 Apr 2, 2024

Choose a reason for hiding this comment