Skip to content

Commit

Permalink
Audit fixes (#1395)
Browse files Browse the repository at this point in the history
* (Issue No 1)

- Clarify `ComboMath` documentation
- Make test `combo_buy_fails_on_amount_out_below_min` more precise

* Fix `log_ceil` and add extensive tests

* (Issue No 7) Enable overflow checks

* (Issue No 9) Fix incorrect test

* (Issue No 10) Resolve FIXMEs

- Won't fix FIXME asking for better error messages
- Solve FIXME in neo-swaps by adding `try_mutate_exists` to
  `PoolStorage`

* Fix formatting

* (Issue No 13) Remove TODO by avoiding a migration

* (Issue No 13) Remove TODO by keeping low-level types

* (Issue No 13) Remove TODO by keeping low-level types

* (Issue No 13) Remove already fixed TODO

* (Issue No 13) Remove won't fix TODOs

* (Issue No 13) Remove more TODOs, some by implementing `CheckedIncRes`

* (Issue No 13) Remove code quality TODO

* (Issue No 14) Define `SCHEDULE_PRIORITY`

* (Issue No 14) Increase readability

* (Issue No 14) Reuse `r_over_b`

* (Issue No 14) Add clarifying comments

* (Issue No 14) Abstract common math code away

* Add missing file

* (Issue No 2) Replace `force_max_work` with `fuel` parameter

* (Issue 6) Replace `SubmitOrigin` with root

* (Issue Nos. 5 & 6) Squashed commit of the following:

commit 4517504
Author: Malte Kliemann <[email protected]>
Date:   Tue Dec 10 18:51:24 2024 +0100

    Benchmark `DecisionMarketOracle`

commit 5832f0b
Author: Malte Kliemann <[email protected]>
Date:   Tue Dec 10 16:55:48 2024 +0100

    Implement `DecisionMarketOracle` using scoreboard

commit e684d47
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 21:24:47 2024 +0100

    Add `BlockNumber` parameter to `update`

commit 5f51378
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 21:17:20 2024 +0100

    Update oracles on each block

commit 2b831fb
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 20:40:26 2024 +0100

    Implement `try_mutate_all`

commit 59cb185
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 18:24:56 2024 +0100

    Use `ProposalStorage` and `MaxProposals`

commit 58afe87
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 17:56:53 2024 +0100

    Add `MaxProposals` value

commit 22068d8
Author: Malte Kliemann <[email protected]>
Date:   Mon Dec 9 14:15:33 2024 +0100

    Add `update` function to `FutarchyOracle` trait

* Fix audit decompressor security miscellaneous comments (#1402)

* Update arkworks-rs dependencies

* Extend get_collection_id comment

* Distinguish collection id generation error

* Use immutable transformations

* fmt

* Update copyrights

* Use retrieval instead of generation for collection id error

* Fix misprint

* fix clippy

* Merge oracles into audit fixes (#1399)

* Add `update` function to `FutarchyOracle` trait

* Add `MaxProposals` value

* Use `ProposalStorage` and `MaxProposals`

* Implement `try_mutate_all`

* Update oracles on each block

* Add `BlockNumber` parameter to `update`

* Implement `DecisionMarketOracle` using scoreboard

* Benchmark `DecisionMarketOracle`

---------

Co-authored-by: Chralt <[email protected]>
  • Loading branch information
maltekliemann and Chralt98 authored Jan 30, 2025
1 parent 33a736e commit d70e33a
Show file tree
Hide file tree
Showing 60 changed files with 1,492 additions and 661 deletions.
338 changes: 261 additions & 77 deletions Cargo.lock

Large diffs are not rendered by default.

24 changes: 22 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ url = "2.5.0"

# Other (wasm)
arbitrary = { version = "1.3.2", default-features = false }
ark-bn254 = { version = "0.4.0", default-features = false, features = ["curve"] }
ark-ff = { version = "0.4.0", default-features = false }
ark-bn254 = { version = "0.5.0", default-features = false, features = ["curve"] }
ark-ff = { version = "0.5.0", default-features = false }
arrayvec = { version = "0.7.4", default-features = false }
cfg-if = { version = "1.0.0" }
fixed = { version = "=1.15.0", default-features = false, features = ["num-traits"] }
Expand All @@ -291,6 +291,15 @@ rand_chacha = { version = "0.3.1", default-features = false }
serde = { version = "1.0.198", default-features = false }
typenum = { version = "1.17.0", default-features = false }

[profile.test]
overflow-checks = true

[profile.test.package."*"]
overflow-checks = true

[profile.dev]
overflow-checks = true

[profile.dev.package]
blake2 = { opt-level = 3 }
blake2b_simd = { opt-level = 3 }
Expand Down Expand Up @@ -336,17 +345,28 @@ x25519-dalek = { opt-level = 3 }
yamux = { opt-level = 3 }
zeroize = { opt-level = 3 }

[profile.dev.package."*"]
overflow-checks = true

[profile.production]
codegen-units = 1
incremental = false
inherits = "release"
lto = true
overflow-checks = true

[profile.production.package."*"]
overflow-checks = true

[profile.release]
opt-level = 3
overflow-checks = true
# Zeitgeist runtime requires unwinding.
panic = "unwind"

[profile.release.package."*"]
overflow-checks = true


# xcm-emulator incompatible block number type fixed
# Commits:
Expand Down
3 changes: 2 additions & 1 deletion primitives/src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ use serde::{Deserialize, Serialize};
pub enum Asset<MarketId> {
CategoricalOutcome(MarketId, CategoryIndex),
ScalarOutcome(MarketId, ScalarPosition),
CombinatorialToken(CombinatorialId),
CombinatorialOutcomeLegacy, // Here to avoid having to migrate all holdings on the chain.
PoolShare(PoolId),
#[default]
Ztg,
ForeignAsset(u32),
ParimutuelShare(MarketId, CategoryIndex),
CombinatorialToken(CombinatorialId),
}

#[cfg(feature = "runtime-benchmarks")]
Expand Down
17 changes: 17 additions & 0 deletions primitives/src/math/checked_ops_res.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ where
fn checked_rem_res(&self, other: &Self) -> Result<Self, DispatchError>;
}

pub trait CheckedIncRes
where
Self: Sized,
{
fn checked_inc_res(&self) -> Result<Self, DispatchError>;
}

impl<T> CheckedAddRes for T
where
T: CheckedAdd,
Expand Down Expand Up @@ -123,3 +130,13 @@ where
self.checked_rem(other).ok_or(DispatchError::Arithmetic(ArithmeticError::DivisionByZero))
}
}

impl<T> CheckedIncRes for T
where
T: CheckedAdd + From<u8>,
{
#[inline]
fn checked_inc_res(&self) -> Result<Self, DispatchError> {
self.checked_add(&1u8.into()).ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow))
}
}
2 changes: 2 additions & 0 deletions primitives/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

mod combinatorial_tokens_api;
mod combinatorial_tokens_benchmark_helper;
mod combinatorial_tokens_fuel;
mod combinatorial_tokens_unsafe_api;
mod complete_set_operations_api;
mod deploy_pool_api;
Expand All @@ -36,6 +37,7 @@ mod zeitgeist_asset;

pub use combinatorial_tokens_api::*;
pub use combinatorial_tokens_benchmark_helper::*;
pub use combinatorial_tokens_fuel::*;
pub use combinatorial_tokens_unsafe_api::*;
pub use complete_set_operations_api::*;
pub use deploy_pool_api::*;
Expand Down
8 changes: 6 additions & 2 deletions primitives/src/traits/combinatorial_tokens_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,26 @@
// You should have received a copy of the GNU General Public License
// along with Zeitgeist. If not, see <https://www.gnu.org/licenses/>.

use crate::types::SplitPositionDispatchInfo;
use crate::{traits::CombinatorialTokensFuel, types::SplitPositionDispatchInfo};
use alloc::vec::Vec;
use core::fmt::Debug;
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
use sp_runtime::DispatchError;

pub trait CombinatorialTokensApi {
type AccountId;
type Balance;
type CombinatorialId;
type MarketId;
type Fuel: Clone + CombinatorialTokensFuel + Debug + Decode + Encode + Eq + TypeInfo;

fn split_position(
who: Self::AccountId,
parent_collection_id: Option<Self::CombinatorialId>,
market_id: Self::MarketId,
partition: Vec<Vec<bool>>,
amount: Self::Balance,
force_max_work: bool,
force_max_work: Self::Fuel,
) -> Result<SplitPositionDispatchInfo<Self::CombinatorialId, Self::MarketId>, DispatchError>;
}
10 changes: 10 additions & 0 deletions primitives/src/traits/combinatorial_tokens_fuel.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// A trait for keeping track of a certain amount of work to be done.
pub trait CombinatorialTokensFuel {
/// Creates a `Fuel` object from a `total` value which indicates the total amount of work to be
/// done. This is usually done for benchmarking purposes.
fn from_total(total: u32) -> Self;

/// Returns a `u32` which indicates the total amount of work to be done. Must be `O(1)` to avoid
/// excessive calculation if this call is used when calculating extrinsic weight.
fn total(&self) -> u32;
}
5 changes: 5 additions & 0 deletions primitives/src/traits/futarchy_oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@
use frame_support::pallet_prelude::Weight;

pub trait FutarchyOracle {
type BlockNumber;

/// Evaluates the query at the current block and returns the weight consumed and a `bool`
/// indicating whether the query evaluated positively.
fn evaluate(&self) -> (Weight, bool);

/// Updates the oracle's data and returns the weight consumed.
fn update(&mut self, now: Self::BlockNumber) -> Weight;
}
4 changes: 3 additions & 1 deletion runtime/battery-station/src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,10 @@ parameter_types! {
/// can lead to extrinsic with very big weight: see delegate for instance.
pub const MaxVotes: u32 = 100;
/// The maximum number of public proposals that can exist at any time.
pub const MaxProposals: u32 = 100;
pub const DemocracyMaxProposals: u32 = 100;

// Futarchy
pub const FutarchyMaxProposals: u32 = 4;
pub const MinDuration: BlockNumber = 7 * BLOCKS_PER_DAY;

// Hybrid Router parameters
Expand Down Expand Up @@ -456,6 +457,7 @@ parameter_type_with_key! {
match currency_id {
Asset::CategoricalOutcome(_,_) => ExistentialDeposit::get(),
Asset::CombinatorialToken(_) => ExistentialDeposit::get(),
Asset::CombinatorialOutcomeLegacy => ExistentialDeposit::get(),
Asset::PoolShare(_) => ExistentialDeposit::get(),
Asset::ScalarOutcome(_,_) => ExistentialDeposit::get(),
#[cfg(feature = "parachain")]
Expand Down
12 changes: 8 additions & 4 deletions runtime/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ macro_rules! decl_common_types {
generic, DispatchError, DispatchResult, RuntimeDebug, SaturatedConversion,
};
use zeitgeist_primitives::traits::{DeployPoolApi, DistributeFees, MarketCommonsPalletApi};
use zrml_combinatorial_tokens::types::CryptographicIdManager;
use zrml_combinatorial_tokens::types::{CryptographicIdManager, Fuel};
use zrml_neo_swaps::types::DecisionMarketOracle;

#[cfg(feature = "try-runtime")]
Expand Down Expand Up @@ -817,7 +817,7 @@ macro_rules! impl_config_traits {
type PalletsOrigin = OriginCaller;
type MaxVotes = MaxVotes;
type WeightInfo = weights::pallet_democracy::WeightInfo<Runtime>;
type MaxProposals = MaxProposals;
type MaxProposals = DemocracyMaxProposals;
type Preimages = Preimage;
type MaxBlacklisted = ConstU32<100>;
type MaxDeposits = ConstU32<100>;
Expand Down Expand Up @@ -1162,6 +1162,7 @@ macro_rules! impl_config_traits {
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = PredictionMarketsCombinatorialTokensBenchmarkHelper<Runtime>;
type CombinatorialIdManager = CryptographicIdManager<MarketId, Blake2_256>;
type Fuel = Fuel;
type MarketCommons = MarketCommons;
type MultiCurrency = AssetManager;
type Payout = PredictionMarkets;
Expand Down Expand Up @@ -1200,11 +1201,11 @@ macro_rules! impl_config_traits {
impl zrml_futarchy::Config for Runtime {
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = DecisionMarketBenchmarkHelper<Runtime>;
type MaxProposals = FutarchyMaxProposals;
type MinDuration = MinDuration;
type Oracle = DecisionMarketOracle<Runtime>;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type SubmitOrigin = EnsureRoot<AccountId>;
type WeightInfo = zrml_futarchy::weights::WeightInfo<Runtime>;
}

Expand Down Expand Up @@ -2288,7 +2289,7 @@ macro_rules! create_common_tests {
};
use zrml_futarchy::types::Proposal;
use zrml_market_commons::types::MarketBuilder;
use zrml_neo_swaps::types::DecisionMarketOracle;
use zrml_neo_swaps::types::{DecisionMarketOracle, DecisionMarketOracleScoreboard};

#[test]
fn futarchy_schedules_and_executes_call() {
Expand Down Expand Up @@ -2362,10 +2363,13 @@ macro_rules! create_common_tests {
};
let call =
Preimage::bound(RuntimeCall::from(remark_dispatched_as)).unwrap();
let scoreboard =
DecisionMarketOracleScoreboard::new(40_000, 10_000, one / 7, one);
let oracle = DecisionMarketOracle::new(
market_id,
Asset::CategoricalOutcome(market_id, 0),
Asset::CategoricalOutcome(market_id, 1),
scoreboard,
);
let when = duration + 10;
let proposal = Proposal { when, call, oracle };
Expand Down
4 changes: 3 additions & 1 deletion runtime/zeitgeist/src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,10 @@ parameter_types! {
/// can lead to extrinsic with very big weight: see delegate for instance.
pub const MaxVotes: u32 = 100;
/// The maximum number of public proposals that can exist at any time.
pub const MaxProposals: u32 = 100;
pub const DemocracyMaxProposals: u32 = 100;

// Futarchy
pub const FutarchyMaxProposals: u32 = 4;
pub const MinDuration: BlockNumber = 7 * BLOCKS_PER_DAY;

// Hybrid Router parameters
Expand Down Expand Up @@ -456,6 +457,7 @@ parameter_type_with_key! {
match currency_id {
Asset::CategoricalOutcome(_,_) => ExistentialDeposit::get(),
Asset::CombinatorialToken(_) => ExistentialDeposit::get(),
Asset::CombinatorialOutcomeLegacy => ExistentialDeposit::get(),
Asset::PoolShare(_) => ExistentialDeposit::get(),
Asset::ScalarOutcome(_,_) => ExistentialDeposit::get(),
#[cfg(feature = "parachain")]
Expand Down
Loading

0 comments on commit d70e33a

Please sign in to comment.