Incorrect behavior of BridgedInflationCurve #57

New Issue

str3tch · 2025-08-10T22:29:32+02:00

str3tch commented

2025-08-10 22:29:32 +02:00

The Problem

During the recent redeployment of the ghost-slow-clap functionality, we observed unusual behavior during the sixth era. The total accumulated commissions reached approximately 1.3 CSPR, yet no distributions occurred at the end of the 6th era. Specifically, the era_payout returned (0, 0) and did not failed.

A special thanks to the ghostie known as Sauron for bringing this to our attention.

Findings

The only way this issue could occur is due to an overflow or underflow in checked_mul or checked_div, which caused a fallback to the None branch, resulting in the return of two Default::default() values, equating to 0 for the current type Balance = u128. You can check this by your hand here.

After a thorough investigation, we identified two key issues:

BridgedImbalance is not being nullified, while AccumulatedCommission is.
An overflow occurs during the computation of the payout.

Nullification

The purpose of BridgedImbalance and AccumulatedCommission is to track imbalances and commissions throughout the era, as we need to eliminate the impact of bridging on the current validators. This is crucial because validators are incentivized by the reward curve to achieve the so-called ideal ratio of staked coins. During the bridging process, after a successful applause, the number of coins should either be minted or burned, which directly affects the total_issuance and consequently alters the current ratio of staked coins.

To obtain the initial total_issuance that remains unaffected by bridging actions in each era, we need to recalculate it in the adjusted total issuance. At the end of the era finalization, it is essential to nullify not only AccumulatedCommissions but BridgedImbalance too in ordre to make sure the adjusted total issuance will not stuck.

Overflow

Since staking relies on the PiecewiseLinear trait, there is a function that defines:

impl<'a> PiecewiseLinear<'a> {
	/// Compute `f(n/d)*d` with `n <= d`. This is useful to avoid loss of precision.
	pub fn calculate_for_fraction_times_denominator<N>(&self, n: N, d: N) -> N
	where
		N: AtLeast32BitUnsigned + Clone,
	{
        // some code here
    }
}

As you can see, we are calling it here. However, instead of multiplying the result by d at the end, we need to multiply it by accumulated_commission. Therefore, we should multiply by accumulated_commission and then divide by adjusted_total_issuance (actually d for the function above) to make sure that reward is dependant on accumulated_commission not on total_issuance.

As an example, what occured in 6th era:

total_staked = 276 * 1e18
total_issuance = 402 * 1e18

# for demonstration purposes let's round computations to make it easier
calculate_for_fraction_times_denominator = 69% * 402 * 1e18 ~ 276 * 1e18
# then we multiply it by accumulated commissions ~1.3 CSPR
checked_mul = 276 * 1e18 * 1.3 * 1e18 ~ 358.8 * 1e36 ~ 3.588 * 1e38

This is clearly an overflow of u128, where u128::MAX is approximately 3.4028237e+38. As a result, we received zero as the payout. This situation is absolutely unacceptable, especially since we can expect commissions to be significantly higher in the future.

Fixes

The solution for the Nullification issue is to reset BridgedImbalance to its default value during the call to nullify_commission at the end of the era finalization. You can find how it was fixed here.
To address the Overflow issue with payouts, we have implemented the MulDiv data structure, which offers phantom overflow protection. This approach is inspired by techniques used for handling large numbers in Solidity (for example, this post). You can find the actual implementation here.

Bug Bounty

We are 99.999% confident in the current implementation of MulDiv; however, if you discover any failing test cases or have suggestions for optimizing the implementation, please share your thoughts in the comments below or reach out through any of our community channels:

Use the latest version from pallet-networks branch and code in pallets/networks folder as a referrence.
Rewards are TBD

## The Problem During the recent redeployment of the `ghost-slow-clap` functionality, we observed unusual behavior during the sixth era. The total accumulated commissions reached approximately `1.3 CSPR,` yet no distributions occurred at the end of the 6th era. Specifically, the `era_payout` returned `(0, 0)` and did not failed. _A special thanks to the ghostie known as **Sauron** for bringing this to our attention._ ## Findings The only way this issue could occur is due to an overflow or underflow in `checked_mul` or c`hecked_div`, which caused a fallback to the `None` branch, resulting in the return of two `Default::default()` values, equating to 0 for the current `type Balance = u128`. You can check this by your hand [here](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/9240f424e1bcf5a18b9ee29ed71e8a639e24e7ba/pallets/networks/src/lib.rs#L107). After a thorough investigation, we identified two key issues: - `BridgedImbalance` is not being nullified, while `AccumulatedCommission` is. - An overflow occurs during the computation of the `payout`. ### Nullification The purpose of `BridgedImbalance` and `AccumulatedCommission` is to track imbalances and commissions throughout the era, as we need to eliminate the impact of bridging on the current validators. This is crucial because validators are incentivized by the [reward curve](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/9240f424e1bcf5a18b9ee29ed71e8a639e24e7ba/runtime/casper/src/lib.rs#L469) to achieve the so-called **ideal ratio** of staked coins. During the bridging process, after a successful **applause**, the number of coins should either be [minted ](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/9240f424e1bcf5a18b9ee29ed71e8a639e24e7ba/pallets/slow-clap/src/lib.rs#L560) or burned, which directly affects the `total_issuance` and consequently alters the current ratio of staked coins. To obtain the initial `total_issuance` that remains unaffected by bridging actions in each era, we need to recalculate it in the adjusted total issuance. At the end of the era finalization, it is essential to nullify not only `AccumulatedCommissions` but `BridgedImbalance` too in ordre to make sure the **adjusted total issuance** will not stuck. ### Overflow Since staking relies on the `PiecewiseLinear` trait, there is a function that defines: ``` impl<'a> PiecewiseLinear<'a> { /// Compute `f(n/d)*d` with `n <= d`. This is useful to avoid loss of precision. pub fn calculate_for_fraction_times_denominator<N>(&self, n: N, d: N) -> N where N: AtLeast32BitUnsigned + Clone, { // some code here } } ``` As you can see, we are calling it [here](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/9240f424e1bcf5a18b9ee29ed71e8a639e24e7ba/pallets/networks/src/lib.rs#L102). However, instead of multiplying the result by `d` at the end, we need to multiply it by `accumulated_commission`. Therefore, we should multiply by `accumulated_commission` and then divide by `adjusted_total_issuance` (actually `d` for the function above) to make sure that reward is dependant on `accumulated_commission` not on `total_issuance`. As an example, what occured in 6th era: ``` total_staked = 276 * 1e18 total_issuance = 402 * 1e18 # for demonstration purposes let's round computations to make it easier calculate_for_fraction_times_denominator = 69% * 402 * 1e18 ~ 276 * 1e18 # then we multiply it by accumulated commissions ~1.3 CSPR checked_mul = 276 * 1e18 * 1.3 * 1e18 ~ 358.8 * 1e36 ~ 3.588 * 1e38 ``` This is clearly an overflow of `u128`, where `u128::MAX` is approximately `3.4028237e+38`. As a result, we received zero as the payout. This situation is absolutely unacceptable, especially since we can expect commissions to be significantly higher in the future. ## Fixes - The solution for the **Nullification issue** is to reset `BridgedImbalance` to its default value during the call to `nullify_commission` at the end of the era finalization. You can find how it was [fixed here]([url](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/d9fa416d937aad5166c37df2079caaf9498c1795/pallets/networks/src/lib.rs#L789)). - To address the **Overflow** issue with payouts, we have implemented the **MulDiv** data structure, which offers _phantom overflow protection_. This approach is inspired by techniques used for handling large numbers in Solidity (for example, [this post](https://medium.com/wicketh/mathemagic-512-bit-division-in-solidity-afa55870a65)). You can find the [actual implementation here](https://git.ghostchain.io/ghostchain/ghost-node/src/commit/d9fa416d937aad5166c37df2079caaf9498c1795/pallets/networks/src/math.rs#L3). ## Bug Bounty We are **99.999%** confident in the current implementation of `MulDiv`; however, if you discover any failing test cases or have suggestions for optimizing the implementation, please share your thoughts in the comments below or reach out through any of our community channels: - [Telegram](https://t.me/realGhostChain) - [X.com (aka. Twitter)](https://x.com/realGhostChain) - [Discord](https://discord.com/invite/CvYP7vrqN3) Use the latest version from `pallet-networks` branch and code in `pallets/networks` folder as a referrence. Rewards are TBD

Sign in to join this conversation.