Skip to content

Commit

Permalink
FIX: Rounding Error (#68)
Browse files Browse the repository at this point in the history
+ Rounding Error is fixed

Before, when we would call the following, it would double count for the
`leftover_rewards` in `rewards_processed`
```rust
            // DAO gets any remainder
            {
                let leftover_rewards = self.ncn_fee_group_rewards(group)?;

                self.route_from_ncn_fee_group_rewards(group, leftover_rewards)?;
                self.route_to_base_fee_group_rewards(BaseFeeGroup::dao(), leftover_rewards)?;
            }
```

This is because we increment `rewards_processed` when we route to the
`ncn_fee_group_rewards` or `base_fee_group_rewards`, however in the
example above, we routed to ncn, counted and then routed to base,
counted.

The fix is to increment `rewards_processed` only when we route from the
reward pool. ( Note, `rewards_processed` is only decremented on the
distribute functions )

This has also been changed in NcnRewardRouter for coherence.
  • Loading branch information
coachchucksol authored Jan 22, 2025
1 parent 3613f86 commit 9d583e9
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 8 deletions.
7 changes: 3 additions & 4 deletions core/src/base_reward_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ impl BaseRewardRouter {
.checked_add(rewards)
.ok_or(TipRouterError::ArithmeticOverflow)?,
);

Ok(())
}

Expand All @@ -499,6 +500,8 @@ impl BaseRewardRouter {
.ok_or(TipRouterError::ArithmeticUnderflowError)?,
);

self.increment_rewards_processed(rewards)?;

Ok(())
}

Expand Down Expand Up @@ -556,8 +559,6 @@ impl BaseRewardRouter {
.ok_or(TipRouterError::ArithmeticOverflow)?,
);

self.increment_rewards_processed(rewards)?;

Ok(())
}

Expand Down Expand Up @@ -602,8 +603,6 @@ impl BaseRewardRouter {
.ok_or(TipRouterError::ArithmeticOverflow)?,
);

self.increment_rewards_processed(rewards)?;

Ok(())
}

Expand Down
7 changes: 3 additions & 4 deletions core/src/ncn_reward_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,8 @@ impl NcnRewardRouter {
.ok_or(TipRouterError::ArithmeticUnderflowError)?,
);

self.increment_rewards_processed(rewards)?;

Ok(())
}

Expand Down Expand Up @@ -525,13 +527,12 @@ impl NcnRewardRouter {
return Ok(());
}

self.increment_rewards_processed(rewards)?;

self.operator_rewards = PodU64::from(
self.operator_rewards()
.checked_add(rewards)
.ok_or(TipRouterError::ArithmeticOverflow)?,
);

Ok(())
}

Expand Down Expand Up @@ -568,8 +569,6 @@ impl NcnRewardRouter {
return Ok(());
}

self.increment_rewards_processed(rewards)?;

for vault_reward in self.vault_reward_routes.iter_mut() {
if vault_reward.vault().eq(vault) {
vault_reward.increment_rewards(rewards)?;
Expand Down
6 changes: 6 additions & 0 deletions integration_tests/tests/fixtures/test_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,8 @@ impl TestBuilder {

// route rewards
tip_router_client.do_route_base_rewards(ncn, epoch).await?;
// Should be able to route twice
tip_router_client.do_route_base_rewards(ncn, epoch).await?;

let base_reward_router = tip_router_client.get_base_reward_router(ncn, epoch).await?;

Expand Down Expand Up @@ -934,6 +936,10 @@ impl TestBuilder {
let operator = operator_root.operator_pubkey;

for group in NcnFeeGroup::all_groups().iter() {
tip_router_client
.do_route_ncn_rewards(*group, ncn, operator, epoch)
.await?;
// Should be able to route twice
tip_router_client
.do_route_ncn_rewards(*group, ncn, operator, epoch)
.await?;
Expand Down
66 changes: 66 additions & 0 deletions integration_tests/tests/tip_router/distribute_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,72 @@ mod tests {

use crate::fixtures::{test_builder::TestBuilder, TestResult};

#[tokio::test]
async fn test_remainder_rewards() -> TestResult<()> {
let mut fixture = TestBuilder::new().await;
let mut tip_router_client = fixture.tip_router_client();

const AMOUNT_TO_ROUTE_SUCH_THAT_REMAINDER: u64 = 10_000;

// Setup with 2 operators for interesting reward splits
// 10% Operator fee
let test_ncn = fixture.create_initial_test_ncn(3, 2, Some(1000)).await?;

///// TipRouter Setup /////
fixture.warp_slot_incremental(1000).await?;

let dao_wallet = Keypair::new();
let dao_wallet_address = dao_wallet.pubkey();
tip_router_client.airdrop(&dao_wallet_address, 1.0).await?;

// Configure fees: 30% block engine, 27% DAO fee, 1.5% NCN fee
tip_router_client
.do_set_config_fees(
Some(300), // block engine fee = 3%
None,
Some(dao_wallet_address), // DAO wallet
Some(270), // DAO fee = 2.7%
None,
Some(15), // NCN fee = .15%
&test_ncn.ncn_root,
)
.await?;

fixture
.warp_slot_incremental(DEFAULT_SLOTS_PER_EPOCH * 2)
.await?;

let ncn = test_ncn.ncn_root.ncn_pubkey;
let epoch = fixture.clock().await.epoch;

fixture.snapshot_test_ncn(&test_ncn).await?;
fixture.vote_test_ncn(&test_ncn).await?;

let valid_slots_after_consensus = {
let config = tip_router_client.get_ncn_config(ncn).await?;
config.valid_slots_after_consensus()
};

fixture
.warp_slot_incremental(valid_slots_after_consensus + 1)
.await?;

fixture.add_routers_for_test_ncn(&test_ncn).await?;

let (base_reward_receiver, _, _) =
BaseRewardReceiver::find_program_address(&jito_tip_router_program::id(), &ncn, epoch);

tip_router_client
.airdrop_lamports(&base_reward_receiver, AMOUNT_TO_ROUTE_SUCH_THAT_REMAINDER)
.await?;

//Run twice, if there is an accounting issue it will fail
tip_router_client.do_route_base_rewards(ncn, epoch).await?;
tip_router_client.do_route_base_rewards(ncn, epoch).await?;

Ok(())
}

#[tokio::test]
async fn test_route_and_distribute_base_rewards() -> TestResult<()> {
let mut fixture = TestBuilder::new().await;
Expand Down

0 comments on commit 9d583e9

Please sign in to comment.