feat: implement Djed Tefnut protocol as Djed Shu simplification #45
Conversation
- Add DjedTefnut.sol: simplified version without reserve ratio constraints - Remove sellBothCoins function and related events - Remove linear treasury fee decay mechanism (constant fee) - Preserve dual-oracle logic and core mint/redeem flows - Add comprehensive test suite for Tefnut-specific functionality - Add deployment script for Tefnut contracts - Update foundry.toml for compilation compatibility Resolves: DjedAlliance#37
WalkthroughA new DjedTefnut contract is introduced as a simplified variant of DjedShu, eliminating reserve ratio constraints, the sellBothCoins function, and linear treasury fee decay while preserving dual-oracle logic and core trading flows. Deployment infrastructure and comprehensive test coverage accompany the implementation, with Foundry configuration updated for IR-based compilation. Changes
Sequence DiagramsequenceDiagram
actor User
participant DjedTefnut
participant Oracle
participant StableCoin
participant Treasury
participant UI as UI Recipient
User->>DjedTefnut: buyStableCoins(receiver, feeUI, ui) {value: msg.value}
DjedTefnut->>Oracle: updatePrice() / getPriceMin() / getPriceMax()
Oracle-->>DjedTefnut: min_price, max_price
rect rgb(200, 220, 240)
Note over DjedTefnut: Calculate pricing<br/>R = reserves, L = liabilities, E = equity<br/>scMinPrice, scMaxPrice from oracle prices
end
DjedTefnut->>DjedTefnut: deductFees(msg.value, feeUI, ui)
DjedTefnut->>Treasury: transfer(treasuryFee portion)
DjedTefnut->>UI: transfer(UI fee portion)
rect rgb(220, 240, 200)
Note over DjedTefnut: Validate constraints<br/>tx limit, supply threshold
end
DjedTefnut->>StableCoin: mint(amountSC) to receiver
DjedTefnut->>User: emit BoughtStableCoins event
Note over DjedTefnut: Similar flows for sell/buy/sell operations<br/>with appropriate minting/burning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
scripts/deployDjedTefnutContract.sol (2)
45-54: Unusedversionparameter.The
versionparameter is declared but never used in the function body. Either implement version-specific configurations or remove the parameter to avoid confusion.function getTefnutConfigFromNetwork( - SupportedNetworks network, - SupportedVersion version + SupportedNetworks network ) internal pure
20-20: Typo in variable name:THREASHOLD_SUPPLY_SC.The variable should be
THRESHOLD_SUPPLY_SC(missing 'H'). While this doesn't affect functionality, it reduces code clarity.- uint256 THREASHOLD_SUPPLY_SC, + uint256 THRESHOLD_SUPPLY_SC,src/test/DjedTefnut.t.sol (1)
186-197: Dual oracle test may not be effective.The test calls
increasePrice()thendecreasePrice()which may cancel each other out, potentially not creating meaningful price divergence. Consider verifying the actual price values differ or setting specific prices to properly test the dual-oracle behavior.function testDualOraclePrices() public { - // Set oracle prices - oracle.increasePrice(); // Increase max price - oracle.decreasePrice(); // Decrease min price - + // Set divergent oracle prices to test dual-oracle behavior + oracle.increasePrice(); // Increase max price + oracle.increasePrice(); // Further increase + uint256 maxPrice = djed.scMaxPrice(0); uint256 minPrice = djed.scMinPrice(0); - - // Max price should be >= min price - assertTrue(maxPrice >= minPrice); + + // Verify prices are different (dual-oracle creates spread) + assertTrue(maxPrice >= minPrice, "Max price should be >= min price"); + // Consider asserting actual divergence: assertTrue(maxPrice > minPrice); }src/DjedTefnut.sol (2)
136-144: Consider skipping transfer whenuiis zero address.When
uiisaddress(0), the transfer still executes, wasting gas. While the low-level call to zero address doesn't fail, it's unnecessary.function deductFees(uint256 value, uint256 feeUI, address ui) internal returns (uint256) { uint256 f = (value * fee) / scalingFactor; uint256 fUI = (value * feeUI) / scalingFactor; uint256 fT = (value * treasuryFee()) / scalingFactor; transfer(treasury, fT); - transfer(ui, fUI); + if (ui != address(0) && fUI > 0) { + transfer(ui, fUI); + } // transfer(address(this), f); // this happens implicitly return value - f - fUI - fT; }
66-68: Stale comment references removed function.The comment "sell both coin -> min price" refers to
sellBothCoinswhich was removed in this PR. Consider updating or removing this comment.function L(uint256 _scPrice) internal view returns (uint256) { return (stableCoin.totalSupply() * _scPrice) / scDecimalScalingFactor; - } // sell both coin -> min price + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
TODO.md(1 hunks)foundry.toml(1 hunks)scripts/deployDjedTefnutContract.sol(1 hunks)src/DjedTefnut.sol(1 hunks)src/test/DjedTefnut.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: negimox
Repo: DjedAlliance/Djed-Solidity PR: 41
File: src/DjedTefnut.sol:214-217
Timestamp: 2025-12-13T17:46:15.360Z
Learning: In Djed protocol implementations (e.g., DjedTefnut.sol), push payments are used intentionally to maintain consistency with the original Djed protocol architecture, rather than pull payment patterns.
📚 Learning: 2025-12-13T17:46:15.360Z
Learnt from: negimox
Repo: DjedAlliance/Djed-Solidity PR: 41
File: src/DjedTefnut.sol:214-217
Timestamp: 2025-12-13T17:46:15.360Z
Learning: In src/DjedTefnut.sol (Djed protocol implementation), ensure that payments are pushed rather than pulled to align with the original Djed architecture. This pattern should be maintained for consistency with the protocol design; avoid introducing pull payment patterns in this file unless there is a clear, documented justification and impact analysis.
Applied to files:
src/DjedTefnut.sol
🪛 LanguageTool
TODO.md
[style] ~46-~46: This phrase is redundant. Consider writing “eliminated”.
Context: .... ✅ Removed sellBothCoins function: Completely eliminated the sellBothCoins() function and `Sol...
(COMPLETELY_ANNIHILATE)
🔇 Additional comments (5)
foundry.toml (1)
8-8: LGTM!Enabling
via_iris appropriate for handling stack depth issues with complex contracts like DjedTefnut. This may increase compilation time but provides better optimization and resolves potential "stack too deep" errors.TODO.md (1)
1-90: Documentation looks comprehensive.This implementation plan clearly documents the changes made, features removed, and acceptance criteria status. Consider moving this to a more permanent location (e.g., as a PR description or ADR) once the PR is merged, as
TODO.mdfiles can become stale.src/test/DjedTefnut.t.sol (1)
28-49: Test setup looks good.The test properly initializes the contract with representative parameters and verifies invariants in
setUp(). The use ofcheats.dealfor test accounts ensures adequate test ETH.src/DjedTefnut.sol (2)
191-194: Push payment pattern is intentional.The use of low-level
callfor ETH transfers aligns with the Djed protocol architecture that intentionally uses push payments. Based on learnings, this pattern should be maintained for consistency with the original design.
84-93: Trading functions look correct.The
buyStableCoinsfunction properly:
- Updates oracle before price calculation
- Deducts fees before minting
- Enforces transaction limits with threshold bypass
- Validates non-zero output
- Uses
nonReentrantguardThe same pattern is consistently applied across all trading functions.
| if (network == SupportedNetworks.ETHEREUM_CLASSIC_MAINNET) { | ||
| return ( | ||
| 0x8Bd4A5F6a4727Aa4AC05f8784aACAbE2617e860A, // Using same as Mordor for now | ||
| 0xBC80a858F6F9116aA2dc549325d7791432b6c6C4, // treasury address | ||
| 1e24, // SCALING_FACTOR | ||
| 25e20, // INITIAL_TREASURY_FEE | ||
| 12500e18, // FEE | ||
| 10e6, // THREASHOLD_SUPPLY_SC | ||
| 1e15, // RESERVE_COIN_MINIMUM_PRICE | ||
| 1e18, // RESERVE_COIN_INITIAL_PRICE | ||
| 1e10 // TX_LIMIT | ||
| ); | ||
| } |
There was a problem hiding this comment.
Mainnet configuration uses testnet addresses.
The ETHEREUM_CLASSIC_MAINNET case uses the same oracle and treasury addresses as Mordor testnet. Deploying to mainnet with these placeholder addresses could result in loss of funds or broken functionality.
Consider:
- Implementing proper mainnet addresses before any mainnet deployment
- Adding a revert for mainnet until production addresses are finalized:
if (network == SupportedNetworks.ETHEREUM_CLASSIC_MAINNET) {
- return (
- 0x8Bd4A5F6a4727Aa4AC05f8784aACAbE2617e860A, // Using same as Mordor for now
- 0xBC80a858F6F9116aA2dc549325d7791432b6c6C4, // treasury address
- ...
- );
+ revert("Mainnet addresses not yet configured");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (network == SupportedNetworks.ETHEREUM_CLASSIC_MAINNET) { | |
| return ( | |
| 0x8Bd4A5F6a4727Aa4AC05f8784aACAbE2617e860A, // Using same as Mordor for now | |
| 0xBC80a858F6F9116aA2dc549325d7791432b6c6C4, // treasury address | |
| 1e24, // SCALING_FACTOR | |
| 25e20, // INITIAL_TREASURY_FEE | |
| 12500e18, // FEE | |
| 10e6, // THREASHOLD_SUPPLY_SC | |
| 1e15, // RESERVE_COIN_MINIMUM_PRICE | |
| 1e18, // RESERVE_COIN_INITIAL_PRICE | |
| 1e10 // TX_LIMIT | |
| ); | |
| } | |
| if (network == SupportedNetworks.ETHEREUM_CLASSIC_MAINNET) { | |
| revert("Mainnet addresses not yet configured"); | |
| } |
🤖 Prompt for AI Agents
In scripts/deployDjedTefnutContract.sol around lines 85 to 97 the
ETHEREUM_CLASSIC_MAINNET block is returning Mordor/testnet oracle and treasury
addresses which must not be used on mainnet; replace those placeholder addresses
with the correct production oracle and treasury addresses (and verify the
numerical constants are intended), or if production addresses are not yet
available change this branch to revert (or require(false, "...")) to prevent
accidental mainnet deployments until the proper addresses are set; add a clear
comment indicating why the branch is blocked or which production addresses were
inserted.
| constructor( | ||
| address oracleAddress, uint256 _scalingFactor, | ||
| address _treasury, uint256 _initialTreasuryFee, | ||
| uint256 _fee, uint256 _thresholdSupplySC, uint256 _rcMinPrice, uint256 _rcInitialPrice, uint256 _txLimit | ||
| ) payable { | ||
| stableCoin = new Coin("StableCoin", "SC"); | ||
| reserveCoin = new Coin("ReserveCoin", "RC"); | ||
| scDecimalScalingFactor = 10**stableCoin.decimals(); | ||
| rcDecimalScalingFactor = 10**reserveCoin.decimals(); | ||
| scalingFactor = _scalingFactor; | ||
|
|
||
| treasury = _treasury; | ||
| initialTreasuryFee = _initialTreasuryFee; | ||
|
|
||
| fee = _fee; | ||
| thresholdSupplySC = _thresholdSupplySC; | ||
| rcMinPrice = _rcMinPrice; | ||
| rcInitialPrice = _rcInitialPrice; | ||
| txLimit = _txLimit; | ||
|
|
||
| oracle = IOracleShu(oracleAddress); | ||
| oracle.acceptTermsOfService(); | ||
| } |
There was a problem hiding this comment.
Missing constructor parameter validation.
The constructor accepts critical addresses and parameters without validation. Zero-address oracle or treasury, or zero scaling factor, would cause runtime failures or fund loss.
constructor(
address oracleAddress, uint256 _scalingFactor,
address _treasury, uint256 _initialTreasuryFee,
uint256 _fee, uint256 _thresholdSupplySC, uint256 _rcMinPrice, uint256 _rcInitialPrice, uint256 _txLimit
) payable {
+ require(oracleAddress != address(0), "Invalid oracle address");
+ require(_treasury != address(0), "Invalid treasury address");
+ require(_scalingFactor > 0, "Invalid scaling factor");
+ require(_rcMinPrice > 0, "Invalid RC min price");
+ require(_rcInitialPrice >= _rcMinPrice, "Initial price below minimum");
+
stableCoin = new Coin("StableCoin", "SC");
reserveCoin = new Coin("ReserveCoin", "RC");🤖 Prompt for AI Agents
In src/DjedTefnut.sol around lines 37 to 59, the constructor accepts critical
addresses and numeric parameters without validation; add require checks at the
start of the constructor to guard against address(0) for oracleAddress and
_treasury, ensure _scalingFactor > 0, and validate other critical numeric params
(e.g. _initialTreasuryFee, _fee, _txLimit, _rcMinPrice, _rcInitialPrice,
_thresholdSupplySC) as appropriate (non-negative or >0 per intended invariants);
perform the checks before any state changes or external calls (like
oracle.acceptTermsOfService()), and revert with clear error messages if
validation fails.
| function E(uint256 _scPrice, uint256 _currentPaymentAmount) internal view returns (uint256) { | ||
| return R(_currentPaymentAmount) - L(_scPrice); | ||
| } // rcTargetPrice -> sell RC -> min price | ||
|
|
||
| function E(uint256 _currentPaymentAmount) external view returns (uint256) { | ||
| return E(scMaxPrice(_currentPaymentAmount), _currentPaymentAmount); | ||
| } |
There was a problem hiding this comment.
Potential underflow in equity calculation.
If liabilities L(_scPrice) exceed reserves R(_currentPaymentAmount), this subtraction will revert due to underflow. While Solidity 0.8+ prevents silent underflow, consider whether this should return 0 or have explicit handling when the protocol is under-collateralized.
function E(uint256 _scPrice, uint256 _currentPaymentAmount) internal view returns (uint256) {
- return R(_currentPaymentAmount) - L(_scPrice);
+ uint256 r = R(_currentPaymentAmount);
+ uint256 l = L(_scPrice);
+ return r > l ? r - l : 0;
} // rcTargetPrice -> sell RC -> min priceAlternatively, if under-collateralization should halt operations, add a descriptive require:
require(R(_currentPaymentAmount) >= L(_scPrice), "Protocol under-collateralized");🤖 Prompt for AI Agents
In src/DjedTefnut.sol around lines 74 to 80, the internal E(uint256,uint256)
function subtracts L(_scPrice) from R(_currentPaymentAmount) which can underflow
if liabilities exceed reserves; update the function to explicitly handle
under-collateralization by either (a) clamping the result to zero when R < L and
returning 0, or (b) asserting the invariant with a clear require(R(...) >=
L(...), "Protocol under-collateralized") so the call reverts with a descriptive
message—pick one behavior and implement it consistently (adjust the external
wrapper if needed).
| function containsSelector(bytes memory code, bytes4 selector) internal pure returns (bool) { | ||
| for (uint i = 0; i <= code.length - 4; i++) { | ||
| if (code[i] == selector[0] && code[i+1] == selector[1] && code[i+2] == selector[2] && code[i+3] == selector[3]) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Potential underflow if bytecode length is less than 4.
If code.length < 4, the subtraction code.length - 4 will underflow (reverting in Solidity 0.8+). While unlikely for deployed contracts, it's good practice to handle this edge case.
function containsSelector(bytes memory code, bytes4 selector) internal pure returns (bool) {
+ if (code.length < 4) return false;
for (uint i = 0; i <= code.length - 4; i++) {
if (code[i] == selector[0] && code[i+1] == selector[1] && code[i+2] == selector[2] && code[i+3] == selector[3]) {
return true;
}
}
return false;
}🤖 Prompt for AI Agents
In src/test/DjedTefnut.t.sol around lines 76 to 83, the subtraction code.length
- 4 can underflow when code.length < 4; add a guard to handle short bytecode by
returning false if code.length < 4, or change the loop condition to avoid
subtraction (e.g., iterate while i + 4 <= code.length) so the function exits
safely without underflow and still correctly checks for the selector.
1 participant
Resolves: #37
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.