RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

State change without event in ChangeFee

s3mvl4d

Description

  • When a protocol changes critical configuration (like LP fees), it should emit an event capturing the old and new values and the fields affected. This enables transparent off‑chain monitoring, auditing, and reproducible state for indexers (e.g., The Graph, block explorers, compliance systems).

  • The admin function ChangeFee() mutates buyFee and/or sellFee without emitting any event. As a result, off‑chain systems cannot reliably track fee changes over time, and operators lose an auditable trail of configuration updates.

​
// Root cause in the codebase with @> marks to highlight the relevant section
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
@> if(_isBuyFee) buyFee = _buyFee; // <-- state changes without event
@> if(_isSellFee) sellFee = _sellFee; // <-- state changes without event
}
​

Risk

Likelihood: Low

  • Occurs on every invocation of ChangeFee during routine governance/ops.

  • Common in active protocols that tune fees in response to market conditions.

Impact: Low

  • Auditability gap: Indexers and dashboards cannot reconstruct a history of fee changes, undermining transparency and compliance reporting.

  • Operational risk: Bots and automation that react to fee changes (e.g., notifying LPs, adjusting routing) will not trigger, potentially causing inconsistent behavior or user confusion.

Proof of Concept

​
function test_ChangeFee_No_Event_Emitted() public {
// Arrange: capture current fees
(uint24 oldBuy, uint24 oldSell) = rebateHook.getFeeConfig();
uint24 newBuy = oldBuy + 100;
uint24 newSell = oldSell + 200;
​
// Expectation: under current code, no event is emitted, so we can't vm.expectEmit a Change event.
// Act: change both fees
rebateHook.ChangeFee(true, newBuy, true, newSell);
​
// Assert: state changed, but there was no event to track it
(uint24 buyAfter, uint24 sellAfter) = rebateHook.getFeeConfig();
assertEq(buyAfter, newBuy);
assertEq(sellAfter, newSell);
​
// NOTE: There is no event to assert against, demonstrating the observability gap.
}
​

Recommended Mitigation

  • Add a dedicated event (including old and new values) and emit it on every mutation.

​
+ // Add an event capturing granular changes
+ event FeeChanged(
+ bool buyFeeChanged,
+ uint24 oldBuyFee,
+ uint24 newBuyFee,
+ bool sellFeeChanged,
+ uint24 oldSellFee,
+ uint24 newSellFee
+ );
​
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
+ uint24 oldBuy = buyFee;
+ uint24 oldSell = sellFee;
​
- if(_isBuyFee) buyFee = _buyFee;
- if(_isSellFee) sellFee = _sellFee;
+ if (_isBuyFee) {
+ buyFee = _buyFee;
+ }
+ if (_isSellFee) {
+ sellFee = _sellFee;
+ }
+
+ emit FeeChanged(
+ _isBuyFee, oldBuy, buyFee,
+ _isSellFee, oldSell, sellFee
+ );
}
​
Updates

Lead Judging Commences

chaossr Lead Judge
13 days ago
chaossr Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!