Skip to content

Commit ce7a392

Browse files
authored
Merge branch 'master' into feat/add-weight-comparison-task
2 parents e9725b5 + 67ac38b commit ce7a392

12 files changed

Lines changed: 172 additions & 12 deletions

File tree

CLAUDE.md

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,68 @@ Types: `feat`, `fix`, `refactor`, `perf`, `test`, `docs`, `style`, `ci`, `build`
147147

148148
**Branches:** `fix/description` or `feat/description`
149149

150+
## Code comments and docs
151+
152+
Default to **no comment**. Only write one when the *why* is non-obvious — a hidden
153+
invariant, a surprising decision, a workaround. If removing the comment wouldn't
154+
confuse a future reader who can see the code, don't write it.
155+
156+
**Never restate what the code already says.** Well-named identifiers and types are
157+
the documentation. Comments that paraphrase the next line are noise.
158+
159+
### Module / file headers
160+
One paragraph max. State what lives here; don't enumerate every item or describe
161+
the flow step-by-step.
162+
163+
### Struct / enum field docs
164+
Skip the obvious (`pub unstaking: Balance`, `pub voters_count: u32`). Document a
165+
field only when its semantics are surprising — e.g. it stacks instead of replacing,
166+
must match an external balance, doubles as an idempotency signal.
167+
168+
### Error variants
169+
One line each, or none if the name already tells the story. Don't write a
170+
paragraph explaining the policy that produces the error — that belongs at the
171+
check site.
172+
173+
### Extrinsic docs
174+
Follow the Description / Parameters / Emits structure from the "Extrinsic
175+
documentation" section above, but keep the **Description to 1–2 lines plus at
176+
most one short paragraph** for genuinely load-bearing context. In particular:
177+
178+
- Do not enumerate `Error` variants in the Description — the `#[pallet::error]`
179+
enum is the source of truth.
180+
- Do not list internal implementation steps ("locks X, then mints Y, then calls
181+
Z"). The code shows that.
182+
- Keep the *why* of any non-obvious constraint (e.g. "refuses while stHDX is in
183+
circulation — outstanding aTokens would be stranded").
184+
185+
### Trait method docs
186+
One line. If the trait-level doc already explains the contract, leave method
187+
docs out entirely.
188+
189+
### Inline comments inside function bodies
190+
Reserve for:
191+
- Non-obvious invariants the next line relies on.
192+
- Why a defensive branch exists / why an error is intentionally swallowed.
193+
- Why an unusual construct (`drain_prefix(...).count()` to actually drain,
194+
`set_lock` vs `extend_lock`, pre-decrement before an external call) is correct.
195+
196+
Skip:
197+
- Narrating control flow ("// new record: increment voter count" above
198+
`voters_count += 1`).
199+
- Explaining what a well-named helper does at its call site.
200+
- Restating the assertion in the next `ensure!`.
201+
202+
### What to keep
203+
Comments that warn a future reader about something they would otherwise miss:
204+
- "Must match `LockableAToken.sol`'s `freeBalance` check"
205+
- "Saturating math — hooks must never block voting"
206+
- "Pool presence ⇔ allocation has run" (load-bearing idempotency signal)
207+
- "stHDX invariants — verify on AAVE config change: (1)…(2)…"
208+
209+
If in doubt, delete the comment and see if the code still reads. If it does,
210+
leave it out.
211+
150212
## Versioning
151213

152214
- **SemVer** on all crates — bump `Cargo.toml` version on changes

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
cc 22ef4648bbc715696260418b2fb3790628c7788f9258b8b9a4f704dc424ee536 # shrinks to asset_out = AssetReserveState { reserve: 190635865358630188, hub_reserve: 1430674154905521092, shares: 100000000000000000, protocol_shares: 100000000000000000 }, amount = 5307728205712486, asset_fee = Permill(300), imbalance = I129 { value: 8936367638875155, negative: true }
2+
cc 611689fb0af263ea5f079f78b2907a0d86a5a841e52c6ab6d4c781f5ba4b59d0 # shrinks to asset = AssetReserveState { reserve: 2951077894421946163, hub_reserve: 5203031156877507362, shares: 100000000000000000, protocol_shares: 100000000000000000 }, position = Position { amount: 1000000000, shares: 1000000000, price: (1763095357072293632, 1000000000000000000) }
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Seeds for failure cases proptest has generated in the past. It is
2+
# automatically read and these particular cases re-run before any
3+
# novel cases are generated.
4+
#
5+
# It is recommended to check this file in to source control so that
6+
# everyone who runs the test benefits from these saved cases.
7+
cc 25c9d46027af5640b9cce25c1970d5378de64752dd1c9520f5757251db333d27 # shrinks to pool = [AssetReserve { amount: 368323000000, decimals: 6 }, AssetReserve { amount: 380987625000000, decimals: 6 }, AssetReserve { amount: 488969232000000, decimals: 6 }, AssetReserve { amount: 12656000000, decimals: 6 }], amount = 241, amp = 6682, (idx_in, idx_out) = (1, 3)
8+
cc b8acd18b42aad7c2bf2fb1869de4f133a49e05a5fd165865f781cadd006488c9
9+
cc b8ffdc09a03ed41698bc59c46d68585ee05e238c14fd8d53d3b7fb0cedfabe3b
10+
cc 3b86f211644d954ca6a8212758cd01d579c3b1d0cfe5cc615ff25726df7d1fb0
11+
cc 63a30c69dc9a73f7cfc9c21fc4cc75e88026f721c3fa9b2b8eaa514e988e2649
12+
cc 464e37423f75de7afd23eef91ca5b097a1aa5e4f9bd839325571089ef256f924

math/src/omnipool/invariants.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ fn high_asset_state() -> impl Strategy<Value = AssetReserveState<Balance>> {
6060
}
6161

6262
fn trade_amount() -> impl Strategy<Value = Balance> {
63-
1_000_000_000..10000 * ONE
63+
ONE / 10..10000 * ONE
6464
}
6565

6666
fn price() -> impl Strategy<Value = FixedU128> {

math/src/stableswap/tests/invariants.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use proptest::proptest;
77
const D_ITERATIONS: u8 = 128;
88
const Y_ITERATIONS: u8 = 64;
99

10-
const RESERVE_RANGE: (Balance, Balance) = (10_000, 1_000_000_000);
10+
// Floor lower than 30k with the 1B ceiling lets in-pool imbalance exceed ~33_000:1, past the assertion's absolute tolerance.
11+
const RESERVE_RANGE: (Balance, Balance) = (30_000, 1_000_000_000);
1112
const TRADE_RANGE: (Balance, Balance) = (1, 5_000);
1213

1314
fn asset_reserve() -> impl Strategy<Value = Balance> {
@@ -22,10 +23,16 @@ fn amplification() -> impl Strategy<Value = Balance> {
2223
2..10000u128
2324
}
2425

26+
// Sample j from the (size - 1) indices that aren't i, instead of rejecting (i, i)
27+
// after the fact — the old prop_filter approach hit proptest's local-reject ceiling
28+
// at high case counts.
2529
fn trade_pair(size: usize) -> impl Strategy<Value = (usize, usize)> {
26-
(0..size, 0..size)
27-
.prop_filter("cannot be equal", |(i, j)| i != j)
28-
.prop_map(|(i, j)| (i, j))
30+
(0..size).prop_flat_map(move |i| {
31+
(
32+
Just(i),
33+
(0..(size - 1)).prop_map(move |j| if j >= i { j + 1 } else { j }),
34+
)
35+
})
2936
}
3037

3138
fn to_precision(value: Balance, precision: u8) -> Balance {

pallets/omnipool/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "pallet-omnipool"
3-
version = "7.4.0"
3+
version = "7.4.1"
44
authors = ['GalacticCouncil']
55
edition = "2021"
66
license = "Apache-2.0"
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Seeds for failure cases proptest has generated in the past. It is
2+
# automatically read and these particular cases re-run before any
3+
# novel cases are generated.
4+
#
5+
# It is recommended to check this file in to source control so that
6+
# everyone who runs the test benefits from these saved cases.
7+
cc b5615762d1d28f231d3ea961960f65a06479d6e1ddce516f0cd7a77fea7849f0
8+
cc 9786c92da35b3c68727ddd8bdcc569386f36e2eb702520448e63d834cf352759
9+
cc 5d47b539d9721d7d14c1b7ec50dfe1ef3236c1b0c89bdba0d7d6edb7e1e56eb8
10+
cc d5f6d6ad933b479696b936b91148a198b5ceed8701c97d9521c184a2e70b20a8

pallets/omnipool/src/tests/slip_fee.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,3 +979,52 @@ fn on_finalize_clears_with_defensive_check() {
979979
assert!(SlipFeeHubReserveAtBlockStart::<Test>::get(100).is_none());
980980
});
981981
}
982+
983+
// A sizeable buy of a high-priced asset_out, paid for with a low-priced asset_in
984+
// under a large slip cap (40+%), inflates the required LRNA past the discriminant
985+
// threshold of the uncapped sell-side inversion, so the math returns Overflow.
986+
987+
// Since maxSlipFee is 25% on prod, this scenario should never really happen
988+
#[test]
989+
fn buy_should_fail_with_overflow_when_low_priced_asset_in_pays_for_large_high_priced_buy() {
990+
use sp_runtime::{ArithmeticError, DispatchError};
991+
992+
let amount: Balance = 3_622_468_058_278_488;
993+
let stable_price = FixedU128::from_float(0.1);
994+
let stable_reserve: Balance = 100_000_000_000_000_000;
995+
let native_reserve: Balance = 100_000_000_000_000_000;
996+
let token_in_price = FixedU128::from_float(0.1);
997+
let token_in_reserve: Balance = 100_000_000_000_000_000;
998+
let token_out_price = FixedU128::from_float(1.981_763_482_850_658_5);
999+
let token_out_reserve: Balance = 100_000_000_000_000_000;
1000+
let asset_fee = Permill::from_rational(22u32, 1_000u32);
1001+
let protocol_fee = Permill::from_percent(3);
1002+
let max_slip_fee = Permill::from_percent(46);
1003+
1004+
let lp_in: u64 = 200;
1005+
let lp_out: u64 = 300;
1006+
let buyer: u64 = 500;
1007+
1008+
ExtBuilder::default()
1009+
.with_endowed_accounts(vec![
1010+
(Omnipool::protocol_account(), DAI, stable_reserve),
1011+
(Omnipool::protocol_account(), HDX, native_reserve),
1012+
(lp_in, 200, token_in_reserve + 2 * ONE),
1013+
(lp_out, 300, token_out_reserve + 2 * ONE),
1014+
(buyer, 200, amount * 1000 + 200 * ONE),
1015+
])
1016+
.with_registered_asset(200)
1017+
.with_registered_asset(300)
1018+
.with_asset_fee(asset_fee)
1019+
.with_protocol_fee(protocol_fee)
1020+
.with_initial_pool(stable_price, FixedU128::from(1))
1021+
.with_token(200, token_in_price, lp_in, token_in_reserve)
1022+
.with_token(300, token_out_price, lp_out, token_out_reserve)
1023+
.build()
1024+
.execute_with(|| {
1025+
SlipFee::<Test>::put(SlipFeeConfig { max_slip_fee });
1026+
1027+
let result = Omnipool::buy(RuntimeOrigin::signed(buyer), 300, 200, amount, Balance::MAX);
1028+
assert_noop!(result, DispatchError::Arithmetic(ArithmeticError::Overflow));
1029+
});
1030+
}

pallets/omnipool/src/tests/slip_fee_invariants.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use proptest::prelude::*;
55

66
pub const ONE: Balance = 1_000_000_000_000;
77

8-
const BALANCE_RANGE: (Balance, Balance) = (100_000 * ONE, 10_000_000 * ONE);
8+
// Starting from 1m units as tiny pools push the slip-fee inversion past its discriminant threshold and the math returns Overflow.
9+
const BALANCE_RANGE: (Balance, Balance) = (500_000 * ONE, 10_000_000 * ONE);
910

1011
fn asset_reserve() -> impl Strategy<Value = Balance> {
1112
BALANCE_RANGE.0..BALANCE_RANGE.1

0 commit comments

Comments
 (0)