Implement Combinatorial Betting and Futarchy #1364
Open
+14,061
−856
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
* Revert "New Asset System (#1295)" (#1338) * Revert "New Asset System (#1295)" This reverts commit a956877. * Fix formatting * Update copyright * Remove pallet-assets dependency * Fix fuzz tests * Merge `main` into `develop` (#1345) * Update dependencies to Polkadot v1.1.0 (#1331) * Update dependencies (#1319) * Update dependencies to polkadot-v1.1.0 * Format code * Remove duplicate dependencies * Update zrml-asset-router (#1321) * Update zrml-primitives * Partially update asset-router * Finalize logic adjustments in asset-router * Make asset-router tests compilable * Correct Inspect routing for market assets in Currencies * Directly invoke Inspect API for Currencies * Add tests for remaining Unbalances functions * Update remaining Zeitgeist pallets (#1322) * Update zrml-asset-router (#1321) * Upgrade zrml-market-commons * Upgrade zrml-authorized && use MockBlock instead of MockBlockU32 * Upgrade zrml-court * Upgrade zrml-global-disputes * Upgrade liquidity mining * Upgrade zrml-rikiddo * Upgrade zrml-simple-disputes * Upgrade zrml-styx * Upgrade zrml-orderbook * Upgrade zrml-parimutuel * Upgrade zrml-swaps * Upgrade zrml-prediction-markets * Upgrade zrml-neo-swaps * Upgrade zrml-hybrid-router * Update license headers * Update runtime (#1323) * Update weight files & Runtime enum * Use workspace metadata * Always use serde serialization for asset types * Make battery station standalone runtime compilable * Make benchmark and try-runtime feature compilable * Make BS build with all features * Make parachain tests compile * Partially fix xcm tests * Use safe xcm version 2 * Update Zeitgeist runtime (except xcm tests) * Format code * Remove deprecated comment * Integrate new xcm-emulator (#1324) * Integrate new xcm-emulator environment * Utilize new xcm-emulator interfaces * Spawn relay-para network using patched xcm-emulator * Use proper collator genesis config * Fix Rococo tests * Finalize Battery Station XCM tests * Finalize Zeitgeist XCM tests * Update client (#1327) * Fix rpc and work on client update * Finalize standalone client * Update parachain client * Use same try-runtime subcommand in every case * Update node/src/cli.rs Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update try-runtime* Makefile targets --------- Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Make CI succeed and add migrations (#1329) * Fix rpc and work on client update * Finalize standalone client * Update parachain client * Use same try-runtime subcommand in every case * Satisfy Clippy * Fix benchmarks * Add migrations * Satisfy Clippy * Update moonkit depedencies * Free disk space more aggressively --------- Co-authored-by: Malte Kliemann <mail@maltekliemann.com> * Update spec version, try-runtime Makefile * Fix copyright notices * Fix broken chain state (#1336) * Add `StorageVersion` fix and contrats fix migrations * Don't set pallet-balances' storage version * Remove migrations from pallet-contracts config * Clear storage tries of contracts * Fix migration and info logs in try-runtime * Fix licenses and comments * Fix formatting --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --------- Co-authored-by: Harald Heckmann <mail@haraldheckmann.de> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Merge * Fix benchmark * Fix compiler error * Fix tests and imports * Fix imports (again...) * Fix orderbook benchmarks * Fix fuzz tests * Fix formatting * Fix orderbook fuzz --------- Co-authored-by: Harald Heckmann <mail@haraldheckmann.de> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Update versions to v0.5.3 * Restructure math module * More scaffolding * Implement combinatorial buy math * Implement price calculation for combo * Remove `println!` * Implement equalization * Implement selling * Add tests for combinatorial buying * Add more tests for combinatorial buying * Add tests for equalization * Add more tests/corner cases * Implement full testing, fix critical bug --------- Co-authored-by: Harald Heckmann <mail@haraldheckmann.de> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
maltekliemann
added
the
s:in-progress
* Implement classical buying using combinatorial buys * Calculate classical sells with combinatorial math * Implement `combo_buy` * Implement `combo_sell`
* Fix compiler and clippy issues * Fix formatting * Fix tests
* Scaffold combo pallet * Fix dependencies/features
* More scaffolding * Add `IdManager` trait * Scaffold ID manager * Use generics * WIP * Rough outline of `CryptographicIdManager` implementation * Improve `MaybeToBytes` implementation for `bool` * Implement pseudo-root * Add more tests * Implement `quadratic_residue` and add tests * Partial implementation of decompression algorithm * Loads of cleaning up * Refactor * Clean up `hash_pair` * Simplify interface * Simplify serialization * More cleanup * Clean up and tests * Better `ToBytes` implementation * Abstract `decompressor` tests * Reorganize tests * Minor clean up * More clean up * Test `get_collection_id`, fix bugs * Clean up * Expose `force_max_work` parameter and test it properly * Properly forget dummies * Add more tests * Fix some error handling, docs, and add missing tests * Prettify
* Extend `Config` * wip * Some refactors * Implement splitting tokens * Implement merging tokens
…1370) * . * Use `core::hint` * Replace `halo2curves` dependency with `ark-*` dependency * Fix clippy issues * Fix formatting
* Add mock for zrml-combinatorial-tokens * Add test framework * Add negative tests for `split_token` * Add more tests, fix some bugs, extend `Event` object * Add more tests * Add more integration tests * Add more integration tests * Add more integration tests * More tests * Add merge tests * final tests * fixes
maltekliemann
added
s:review-needed
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1364 +/- ##
==========================================
+ Coverage 93.21% 93.53% +0.32%
==========================================
Files 132 178 +46
Lines 29358 34547 +5189
==========================================
+ Hits 27366 32315 +4949
- Misses 1992 2232 +240
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* Add numerical thresholds to combinatorial betting * Add protected `exp` for normal bets * Ensure correctness of partitions * Check partitions
mergify
bot
added
s:in-progress
* Add licenses * Add remark about LGPL-3.0 licensing by Gnosis
maltekliemann
added
s:review-needed
mergify
bot
added
s:in-progress
maltekliemann
added
s:review-needed
* Fix formatting * Fix mess with copyright notices
mergify
bot
added
s:in-progress
maltekliemann
added
s:review-needed
mergify
bot
added
s:in-progress
maltekliemann
added
s:review-needed
maltekliemann
added
s:review-needed
Chralt98
reviewed
Dec 3, 2024
| @@ -389,7 +515,7 @@ mod pallet { | |||
| /// | |||
| /// # Parameters | |||
| /// | |||
| /// - `market_id`: Identifier for the market related to the pool. | |||
| /// - `pool`: Identifier for the pool to add liquidity to. | |||
There was a problem hiding this comment.
Suggested change
| /// - `pool`: Identifier for the pool to add liquidity to. | |
| /// - `pool_id`: Identifier for the pool to add liquidity to. |
Comment on lines
+872
to
+882
|
|
||
| let buy = vec![asset_in]; | ||
| let keep = vec![]; | ||
| let sell = pool.assets_complement(&buy); | ||
| let amount_out = pool.calculate_swap_amount_out_for_sell( | ||
| buy, | ||
| keep, | ||
| sell, | ||
| amount_in, | ||
| Zero::zero(), | ||
| )?; |
There was a problem hiding this comment.
In function do_sell isn't asset_in sold? If yes, why is buy = [asset_in]?
There was a problem hiding this comment.
It's fine, because asset_in is sold and in order to get the amount of the full set of all possible outcomes, the amount_out is calculated in which all other assets are sold to get an equal amount of each possible asset back, which can then be burned for collateral.
| @@ -644,7 +900,7 @@ mod pallet { | |||
| remaining: amount_out_minus_fees, | |||
| swap_fees: swap_fee_amount, | |||
| external_fees: external_fee_amount, | |||
| } = Self::distribute_fees(market_id, pool, amount_out)?; | |||
| } = Self::distribute_fees(pool, &pool.account_id.clone(), amount_out)?; | |||
There was a problem hiding this comment.
If account is &pool.account_id and inside the distribute_fees is:
T::MultiCurrency::transfer(pool.collateral, account, &pool.account_id, swap_fees)?;then the transfer is from the pool account to the pool account itself?
Comment on lines
1162
to
1171
| let pool = Pool { | ||
| account_id: pool_account_id.clone(), | ||
| assets: market.outcome_assets().try_into().map_err(|_| Error::<T>::Unexpected)?, | ||
| reserves: reserves.clone().try_into().map_err(|_| Error::<T>::Unexpected)?, | ||
| collateral, | ||
| liquidity_parameter, | ||
| liquidity_shares_manager: LiquidityTree::new(who.clone(), amount)?, | ||
| swap_fee, | ||
| pool_type: PoolType::Standard(market_id), | ||
| }; |
| @@ -910,10 +1177,97 @@ mod pallet { | |||
| &pool.account_id, | |||
| T::MultiCurrency::minimum_balance(collateral), | |||
| )?; | |||
| Pools::<T>::insert(market_id, pool); | |||
| let pool_id = <Self as PoolStorage>::add(pool)?; | |||
| MarketIdToPoolId::<T>::insert(market_id, pool_id); | |||
There was a problem hiding this comment.
What is about the old pools. Those don't have an entry in MarketIdToPoolId?
|
This pull request is now in conflicts. Could you fix it @maltekliemann? 🙏 |
mergify
bot
added
s:revision-needed
Chralt98
reviewed
Dec 5, 2024
| sell.clone(), | ||
| amount_in_minus_fees, | ||
| )?; | ||
| let amount_out = swap_amount_out.checked_add_res(&amount_in_minus_fees)?; |
There was a problem hiding this comment.
Why checked_add_res(&amount_in_minus_fees)? As far as I can see amount_out is only being used for the event.
| false, | ||
| )); | ||
|
|
||
| // Alice is left with 1 unit of [0, 0, 1], 2 units of [1, 1, 0] and one unit of each of the |
There was a problem hiding this comment.
Suggested change
| // Alice is left with 1 unit of [0, 0, 1], 2 units of [1, 1, 0] and one unit of each of the | |
| // Alice is left with 2 units of [0, 0, 1], 3 units of [1, 1, 0] and one unit of each of the |
Chralt98
reviewed
Dec 5, 2024
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::Ztg, _99).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::ForeignAsset(1), _99).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::Ztg, _100).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::Ztg, _100).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
Chralt98
reviewed
Dec 5, 2024
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::Ztg, _99).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
Chralt98
reviewed
Dec 5, 2024
| ExtBuilder::build().execute_with(|| { | ||
| let alice = Account::new(0).deposit(Asset::ForeignAsset(1), _99).unwrap(); | ||
|
|
||
| // Market has three outcomes, but there's an element in the partition of size two. |
There was a problem hiding this comment.
Suggested change
| // Market has three outcomes, but there's an element in the partition of size two. |
Chralt98
reviewed
Dec 5, 2024
|
|
||
| #[test_case(vec![B0, B1, B0, B1]; "incorrect_len")] | ||
| #[test_case(vec![B0, B0, B0]; "all_zero")] | ||
| #[test_case(vec![B0, B0, B0]; "all_one")] |
There was a problem hiding this comment.
Suggested change
| #[test_case(vec![B0, B0, B0]; "all_one")] | |
| #[test_case(vec![B1, B1, B1]; "all_one")] |
Chralt98
reviewed
Dec 6, 2024
| } | ||
|
|
||
| #[test] | ||
| fn redeem_position_fails_if_tokens_have_to_value() { |
There was a problem hiding this comment.
Suggested change
| fn redeem_position_fails_if_tokens_have_to_value() { | |
| fn redeem_position_fails_if_tokens_have_no_value() { |
Chralt98
reviewed
Dec 6, 2024
Comment on lines
+34
to
+40
| let try_mutate_result = Proposals::<T>::try_mutate(to_be_scheduled_at, |proposals| { | ||
| proposals.try_push(proposal.clone()).map_err(|_| Error::<T>::CacheFull) | ||
| }); | ||
|
|
||
| Self::deposit_event(Event::<T>::Submitted { duration, proposal }); | ||
|
|
||
| Ok(try_mutate_result?) |
There was a problem hiding this comment.
Suggested change
| let try_mutate_result = Proposals::<T>::try_mutate(to_be_scheduled_at, |proposals| { | |
| proposals.try_push(proposal.clone()).map_err(|_| Error::<T>::CacheFull) | |
| }); | |
| Self::deposit_event(Event::<T>::Submitted { duration, proposal }); | |
| Ok(try_mutate_result?) | |
| Proposals::<T>::try_mutate(to_be_scheduled_at, |proposals| { | |
| proposals.try_push(proposal.clone()).map_err(|_| Error::<T>::CacheFull) | |
| })?; | |
| Self::deposit_event(Event::<T>::Submitted { duration, proposal }); | |
| Ok(()) |
Chralt98
reviewed
Dec 6, 2024
| if result.is_ok() { | ||
| Self::deposit_event(Event::<T>::Scheduled { proposal }); | ||
| } else { | ||
| Self::deposit_event(Event::<T>::UnexpectedSchedulerError); |
There was a problem hiding this comment.
This case is unfortunate. This can obviously lead to the following scenario: A proposal is submitted and stored in Proposals, but because the scheduler has too many scheduled calls at proposal.when, it fails and the whole proposal is removed from the storage, although it has been evaluated to be approved. It might be better to have another extrinsic call that allows approved proposals to be scheduled at the time of the root origins choosing. So essentially this function here could be called evaluate_proposal that either emits Approved and stores a boolean flag approved in the proposal or Rejected that removes the proposal. After the successful scheduling from the root origin, the proposal can be removed too.
Chralt98
reviewed
Dec 6, 2024
| use frame_support::{ | ||
| dispatch::RawOrigin, | ||
| pallet_prelude::Weight, | ||
| traits::schedule::{v3::Anon, DispatchTime}, |
There was a problem hiding this comment.
Suggested change
| traits::schedule::{v3::Anon, DispatchTime}, | |
| traits::schedule::{v3::Anon, DispatchTime, HARD_DEADLINE}, |
Chralt98
reviewed
Dec 6, 2024
| let result = T::Scheduler::schedule( | ||
| DispatchTime::At(proposal.when), | ||
| None, | ||
| 63, |
There was a problem hiding this comment.
Suggested change
| 63, | |
| HARD_DEADLINE, |
Chralt98
reviewed
Dec 6, 2024
Comment on lines
+34
to
+46
| #[benchmark] | ||
| fn submit_proposal() { | ||
| let duration = T::MinDuration::get(); | ||
|
|
||
| let oracle = T::BenchmarkHelper::create_oracle(true); | ||
| let proposal = Proposal { | ||
| when: Default::default(), | ||
| call: Bounded::Inline(vec![7u8; 128].try_into().unwrap()), | ||
| oracle, | ||
| }; | ||
|
|
||
| #[extrinsic_call] | ||
| _(RawOrigin::Root, duration, proposal.clone()); |
There was a problem hiding this comment.
At to_be_scheduled_at there could be 15 existing proposals in the worst case. This isn't accounted here in the benchmark.
Chralt98
reviewed
Dec 6, 2024
Comment on lines
+53
to
+67
| #[benchmark] | ||
| fn maybe_schedule_proposal() { | ||
| let when = u32::MAX.into(); | ||
| let oracle = T::BenchmarkHelper::create_oracle(true); | ||
| let proposal = | ||
| Proposal { when, call: Bounded::Inline(vec![7u8; 128].try_into().unwrap()), oracle }; | ||
|
|
||
| #[block] | ||
| { | ||
| Pallet::<T>::maybe_schedule_proposal(proposal.clone()); | ||
| } | ||
|
|
||
| let expected_event = <T as Config>::RuntimeEvent::from(Event::<T>::Scheduled { proposal }); | ||
| System::<T>::assert_last_event(expected_event.into()); | ||
| } |
There was a problem hiding this comment.
At now in on_initialize there could be 15 existing proposals in the worst case. T::DbWeight::get().reads_writes(1, 1) does not account for storage decoding and encoding.
Chralt98
reviewed
Dec 6, 2024
Comment on lines
+1
to
+16
| // Copyright 2024 Forecasting Technologies LTD. | ||
| // | ||
| // This file is part of Zeitgeist. | ||
| // | ||
| // Zeitgeist is free software: you can redistribute it and/or modify it | ||
| // under the terms of the GNU General Public License as published by the | ||
| // Free Software Foundation, either version 3 of the License, or (at | ||
| // your option) any later version. | ||
| // | ||
| // Zeitgeist is distributed in the hope that it will be useful, but | ||
| // WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
| // General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU General Public License | ||
| // along with Zeitgeist. If not, see <https://www.gnu.org/licenses/>. |
There was a problem hiding this comment.
This file and directory can be removed, since it has no content.
Chralt98
reviewed
Dec 6, 2024
Chralt98
reviewed
Dec 12, 2024
Comment on lines
-51
to
+58
| CombinatorialOutcome, | ||
| CombinatorialToken(CombinatorialId), | ||
| PoolShare(PoolId), | ||
| #[default] | ||
| Ztg, | ||
| ForeignAsset(u32), | ||
| ParimutuelShare(MarketId, CategoryIndex), | ||
| } | ||
| // TODO Needs storage migration |
There was a problem hiding this comment.
Does not need a storage migration, because the enum variant CombinatorialOutcome has never been used before in the storage.
Each enum variant is encoded separately in the storage. https://docs.substrate.io/reference/scale-codec/
The two potential storage items that could contain a CombinatorialOutcome are Markets that contains the base_asset as a potential Asset. The other is Pools. But the Asset CombinatorialOutcome was never assigned.
Chralt98
reviewed
Dec 20, 2024
| @@ -444,7 +573,7 @@ mod pallet { | |||
| /// | |||
| /// # Parameters | |||
| /// | |||
| /// - `market_id`: Identifier for the market related to the pool. | |||
| /// - `poold_id`: Identifier for the pool to withdraw liquidity from. | |||
There was a problem hiding this comment.
Suggested change
| /// - `poold_id`: Identifier for the pool to withdraw liquidity from. | |
| /// - `pool_id`: Identifier for the pool to withdraw liquidity from. |
Chralt98
reviewed
Dec 23, 2024
| if keep.is_empty() { | ||
| ensure!(amount_keep.is_zero(), Error::<T>::InvalidAmountKeep); | ||
| } else { | ||
| ensure!(amount_keep < amount_buy, Error::<T>::InvalidAmountKeep); |
Chralt98
reviewed
Dec 23, 2024
Comment on lines
+1496
to
+1507
| // Ensure that numerical limits of all prices are respected. | ||
| for &asset in pool.assets().iter() { | ||
| let spot_price = pool.calculate_spot_price(asset)?; | ||
| ensure!( | ||
| spot_price >= COMBO_MIN_SPOT_PRICE.saturated_into(), | ||
| Error::<T>::NumericalLimits(NumericalLimitsError::SpotPriceSlippedTooLow) | ||
| ); | ||
| ensure!( | ||
| spot_price <= COMBO_MAX_SPOT_PRICE.saturated_into(), | ||
| Error::<T>::NumericalLimits(NumericalLimitsError::SpotPriceSlippedTooHigh) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This is duplicated from above.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.