feat: Implement Djed Tefnut protocol with ETH backing (Team Qwerty | Unstoppable Hackathon) #38
Conversation
- Add DjedTefnut.sol contract with simplified parameters - Remove reserve ratio constraints (min/max) - Remove sellBothCoins function entirely - Replace treasury fee decay with fixed fee - Preserve dual-oracle logic from DjedShu - Add comprehensive test suite (17 tests passing) - Update deployment scripts and parameters - Update MockShuOracle to implement IOracleShu interface Resolves DjedAlliance#37
WalkthroughAdds a new Djed Tefnut protocol implementation (ETH‑backed) including the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Djed as DjedTefnut
participant Oracle as Oracle (Shu)
participant SC as StableCoin
participant RC as ReserveCoin
participant Treasury as Treasury
rect rgba(220,245,255,0.8)
Note over User,Djed: Buy StableCoin
User->>Djed: buyStableCoins(receiver, feeUI, ui) {ETH}
Djed->>Oracle: readMaxPrice()
Oracle-->>Djed: (maxPrice, lastUpdate)
Djed->>Djed: compute SC amount & fees
Djed->>Treasury: transfer(treasuryFee)
Djed->>User: transfer(UI fee to ui)
Djed->>SC: mint(receiver, amountSC)
Djed-->>User: emit BoughtStableCoins
end
rect rgba(255,235,235,0.8)
Note over User,Djed: Sell StableCoin
User->>Djed: sellStableCoins(amountSC, receiver, feeUI, ui)
Djed->>Oracle: readMinPrice()
Oracle-->>Djed: (minPrice, lastUpdate)
Djed->>Djed: compute ETH payout & fees
Djed->>SC: burn(user, amountSC)
Djed->>Treasury: transfer(treasuryFee)
Djed->>User: transfer(UI fee to ui)
Djed->>User: transfer(ETH payout to receiver)
Djed-->>User: emit SoldStableCoins
end
rect rgba(235,255,235,0.8)
Note over User,Djed: Buy ReserveCoin
User->>Djed: buyReserveCoins(receiver, feeUI, ui) {ETH}
Djed->>Djed: compute RC amount (rcBuyingPrice)
Djed->>Treasury: transfer(treasuryFee)
Djed->>User: transfer(UI fee to ui)
Djed->>RC: mint(receiver, amountRC)
Djed-->>User: emit BoughtReserveCoins
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 1
🧹 Nitpick comments (8)
README.md (1)
146-164: Add language specifier to fenced code block.The code block for Contract Architecture is missing a language specifier. This affects markdown rendering and linting compliance.
-``` +```text src/ ├── Djed.sol # Standard implementationscripts/deployDjedTefnutContract.s.sol (2)
9-25: Consider parameterizing the network selection for consistency.The deployment script hardcodes the network to
ETHEREUM_CLASSIC_MORDOR(line 24), requiring manual code changes for different deployments. Other deployment scripts in this repo (e.g.,deployDjedContract.s.sol,deployDjedShuContract.sol) userun(uint8)with a network parameter.- function run() external { + function run(uint8 networkId) external { uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); + SupportedNetworks network = SupportedNetworks(networkId); // Get configuration for Tefnut deployment ( address oracleAddress, address treasuryAddress, uint256 scalingFactor, uint256 treasuryFeeFixed, uint256 fee, uint256 thresholdSupplySC, uint256 rcMinPrice, uint256 rcInitialPrice, uint256 txLimit ) = getTefnutConfigFromNetwork( - SupportedNetworks.ETHEREUM_CLASSIC_MORDOR // Change network as needed + network );This would align with the README deployment command pattern:
forge script ./scripts/deployDjedTefnutContract.s.sol:DeployDjedTefnutContract --broadcast --rpc-url <NETWORK_RPC_ENDPOINT> --sig "run(uint8)" -- <SupportedNetworks_ID> --verify
40-51: Consider making initial reserve configurable.The initial reserve of
1 etheris hardcoded. For production deployments, this may need to be configurable via environment variable or constructor parameter to accommodate different deployment scenarios.+ uint256 initialReserve = vm.envOr("INITIAL_RESERVE", uint256(1 ether)); + // Deploy DjedTefnut with initial reserve - DjedTefnut djed = new DjedTefnut{value: 1 ether}( + DjedTefnut djed = new DjedTefnut{value: initialReserve}(src/DjedTefnut.sol (2)
35-57: Consider adding constructor parameter validation.The constructor accepts multiple parameters without validation. Invalid parameters could result in a broken deployment. Consider adding checks such as:
treasury != address(0)_scalingFactor > 0_treasuryFeeFixed + _fee < _scalingFactor(total fees < 100%)_rcMinPrice <= _rcInitialPrice_thresholdSupplySC > 0Example validation:
constructor( address oracleAddress, uint256 _scalingFactor, address _treasury, uint256 _treasuryFeeFixed, uint256 _fee, uint256 _thresholdSupplySC, uint256 _rcMinPrice, uint256 _rcInitialPrice, uint256 _txLimit ) payable { + require(_treasury != address(0), "Invalid treasury address"); + require(_scalingFactor > 0, "Invalid scaling factor"); + require(_treasuryFeeFixed + _fee < _scalingFactor, "Total fees exceed 100%"); + require(_rcMinPrice <= _rcInitialPrice, "Min price exceeds initial price"); + require(_thresholdSupplySC > 0, "Invalid threshold"); + stableCoin = new Coin("StableCoin", "SC"); reserveCoin = new Coin("ReserveCoin", "RC");
139-147: Consider adding validation for user-providedfeeUIparameter.The function doesn't validate the
feeUIparameter passed by users. If a user accidentally provides afeeUIvalue that, combined with protocol and treasury fees, exceeds 100% of the transaction value, the function will revert on line 146 due to underflow.While this doesn't create a security risk (user only loses gas), it could lead to poor UX.
Consider adding:
function deductFees(uint256 value, uint256 feeUI, address ui) internal returns (uint256) { + require(feeUI < scalingFactor, "UI fee too high"); uint256 f = (value * fee) / scalingFactor; uint256 fUI = (value * feeUI) / scalingFactor; uint256 fT = (value * treasuryFeeFixed) / scalingFactor; + require(f + fUI + fT < value, "Total fees exceed transaction value");src/test/DjedTefnut.t.sol (3)
48-99: Reserve ratio tests verify operations succeed but could be more comprehensive.The tests correctly verify that buy/sell operations succeed without ratio constraints. However, they could be strengthened by:
- Calculating the actual reserve ratio after operations
- Asserting it's outside the bounds that would have been enforced in DjedShu
- Confirming the operation still succeeded
Current tests achieve the core objective (operations aren't blocked), but explicit ratio calculations would make the tests more robust and self-documenting.
Example enhancement for
testBuyStableCoinsWithoutMinRatioCheck:uint256 reserve = djed.R(0); uint256 liabilities = djed.L(); + // Calculate ratio (reserve / liabilities) with scaling + uint256 ratio = (reserve * SCALING_FACTOR) / liabilities; + + // In DjedShu, minimum ratio is typically 1.1 (110%) + uint256 oldMinRatio = 11e23; // 1.1 * 1e24 + assertTrue(ratio < oldMinRatio, "Ratio should be below old minimum"); + // In normal Djed, this would fail if reserve/liabilities < 1.1 // In Tefnut, it should work regardless assertTrue(reserve > 0, "Reserve should exist");
161-209: Dual-oracle tests verify basic behavior but have weak assertions.The tests confirm that price changes affect outcomes, but they don't verify the actual amounts or that the correct oracle price (max vs min) is used.
Strengthen assertions by calculating and verifying expected values:
function testUsesMaxPriceForBuyingSC() public { // Increase oracle price oracle.setPrice(2e18); // Now 1 SC = 2 ETH max cheats.deal(alice, 10 ether); cheats.prank(alice); djed.buyStableCoins{value: 10 ether}(alice, 0, address(0)); - // Should use max price, so user gets fewer SC uint256 scBalance = djed.stableCoin().balanceOf(alice); - // With fees deducted, should get approximately 4.8 SC (rough calculation) - // Actual calculation: (10 ETH - fees) / 2 ETH per SC - assertTrue(scBalance > 0, "Should receive SC"); + // Calculate expected: (10 ETH - 2.5% - 1.5%) / 2 ETH = 4.8 SC + uint256 netAmount = 10 ether - (10 ether * (TREASURY_FEE_FIXED + PROTOCOL_FEE)) / SCALING_FACTOR; + uint256 expectedSC = (netAmount * scDecimalScalingFactor) / 2e18; + assertEq(scBalance, expectedSC, "Should receive correct SC amount using max price"); }Similarly,
testOracleUpdateIsCalledshould be removed or properly implemented to actually verify the oracle was updated.
359-369: Remove or properly implement placeholder tests.Both
testNoReserveRatioMinVariableandtestNoTreasuryRevenueTrackingjust asserttruewithout testing anything. These tests don't add value and should either be removed or properly implemented.Since the absence of variables/functions is verified at compile time, these tests might not be necessary. Consider removing them to clean up the test suite.
- function testNoReserveRatioMinVariable() public { - // Try to access reserveRatioMin - should fail at compile time - // This is verified by the contract not having these immutable variables - assertTrue(true, "Contract compiles without ratio variables"); - } - - function testNoTreasuryRevenueTracking() public { - // Verify no treasuryRevenue state variable exists - // In DjedTefnut, we don't track revenue since fee doesn't decay - assertTrue(true, "Contract compiles without revenue tracking"); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
foundry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
README.md(3 hunks)foundry.toml(1 hunks)scripts/DeploymentParameters.sol(3 hunks)scripts/deployDjedTefnutContract.s.sol(1 hunks)src/DjedTefnut.sol(1 hunks)src/mock/MockShuOracle.sol(2 hunks)src/test/DjedTefnut.t.sol(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
146-146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (18)
foundry.toml (1)
8-10: LGTM!The optimizer configuration is appropriate for production deployment.
via_ir = trueenables IR-based compilation which can help with stack-too-deep errors and produce more optimized bytecode.optimizer_runs = 200is a sensible default balancing deployment cost and runtime efficiency.README.md (1)
7-34: Well-documented protocol variants with clear trade-offs.The documentation clearly explains the differences between Djed, Djed Shu, and Djed Tefnut variants. The warning about Tefnut's lack of overcollateralization guarantees (line 189) appropriately highlights the experimental nature.
src/mock/MockShuOracle.sol (1)
6-43: LGTM!The mock oracle correctly implements the
IOracleShuinterface for testing purposes. Returning the same value for bothreadMaxPrice()andreadMinPrice()is appropriate for a mock, and allows tests to verify behavior with controlled price values. ThesetPrice()helper simplifies test setup.scripts/DeploymentParameters.sol (2)
75-77: Good: Oracle selection correctly updated for version-aware deployment.The conditional properly selects the Shu oracle for both
DJED_SHUandDJED_TEFNUTversions on Mordor network, ensuring dual-oracle support is available for Tefnut.
155-156: Consider consistency: Mainnet Tefnut deployment uses standard HebeSwap oracle, while Mordor Tefnut uses the Shu oracle variant.DjedTefnut imports
IOracleShu.soland the codebase definesHEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MORDOR, but no corresponding Shu oracle constant exists for Mainnet. ThegetTefnutConfigFromNetwork()function assigns the standardHEBESWAP_ORACLE_INVERTED_ADDRESS_MAINNETto Mainnet while assigningHEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MORDORto Mordor. If Tefnut's dual-oracle architecture requires Shu-compatible oracles, either: (1) define a Mainnet Shu oracle constant and use it consistently, or (2) document why Mainnet intentionally uses the standard variant.src/DjedTefnut.sol (7)
83-93: Clean implementation with proper safeguards.The function correctly:
- Uses
scMaxPricefor conservative pricing (dual-oracle logic preserved)- Removes reserve ratio constraints as required
- Maintains reentrancy protection
- Enforces transaction limits
- Includes zero-amount validation
95-106: Proper implementation following best practices.The function correctly uses
scMinPricefor redemptions and follows the checks-effects-interactions pattern by burning tokens before transferring funds.
121-133: RC redemption follows checks-effects-interactions pattern.The function correctly removes explicit min ratio checks. Similar to
buyReserveCoins, redemptions will revert if equity is negative (which may be intentional for the equity tranche).
150-152: Fixed treasury fee correctly implemented.The function correctly returns a fixed fee with no decay mechanism, satisfying the PR requirement to "remove the linear decay mechanism of the treasury fee."
157-193: Dual-oracle logic correctly preserved from DjedShu.The price functions properly implement:
readMaxPrice/readMinPricefrom the oracle (lines 165, 170)- Conservative pricing for both SC and RC operations
- Proper handling of edge cases (zero RC supply at line 191)
- Appropriate scaling factor conversions
195-198: Safe ETH transfer implementation.The low-level call with success check is the correct pattern for ETH transfers. All callers have
nonReentrantprotection, preventing reentrancy attacks.
108-119: Implicit underflow prevents RC purchases when system is undercollateralized.The function removes explicit max ratio checks as designed, but
rcTargetPricecallsE(_scPrice, _currentPaymentAmount), which performsR(_currentPaymentAmount) - L(_scPrice). In Solidity 0.8+, this reverts on underflow when liabilities exceed reserve, preventing RC purchases once the system becomes undercollateralized.While this acts as a safety mechanism to prevent RC from having negative backing, it conflicts with the stated design goal of "maximum flexibility" and "no reserve ratio constraints." This implicit constraint is neither documented nor tested. Clarify whether this revert-on-negative-equity behavior is intentional or should be handled explicitly.
src/test/DjedTefnut.t.sol (6)
28-42: Test setup correctly initializes the testing environment.The setup properly instantiates the oracle and DjedTefnut contract with appropriate test parameters and initial reserve.
105-117: Correctly verifiessellBothCoinsfunction removal.The low-level call approach properly tests the absence of the function, satisfying the requirement that it be "fully eliminated."
123-155: Fixed treasury fee tests are thorough and correct.Both tests properly verify:
- Fee remains constant across multiple transactions (no decay)
- Treasury receives the exact fixed fee amount
- Calculations match the expected formula
215-278: Core mint/redeem flow tests adequately verify basic functionality.The tests confirm that:
- SC and RC can be minted and redeemed
- Balances are correctly updated
- Multiple users can interact with the protocol
- ETH is returned on redemption
284-301: UI fee distribution test correctly verifies fee calculation and transfer.The test properly validates that the UI fee is calculated and distributed accurately.
336-353: Deployment and parameter tests are well-designed.These tests properly verify contract initialization and parameter configuration.
- Changed independent if blocks to if/else if/else chain - Added explicit revert for unsupported networks - Prevents returning uninitialized state variables - Applied to both getConfigFromNetwork() and getTefnutConfigFromNetwork()
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/DeploymentParameters.sol(5 hunks)
🔇 Additional comments (4)
scripts/DeploymentParameters.sol (4)
15-16: LGTM!Clean addition of the DJED_TEFNUT variant to support the new protocol version.
71-72: Excellent improvements to control flow and error handling.The else-if chain and explicit revert for unsupported networks address the past review comment and prevent returning uninitialized state variables. The conditional oracle selection for DJED_SHU and DJED_TEFNUT on Mordor correctly preserves the dual-oracle logic.
Also applies to: 84-99
117-146: Well-structured Tefnut configuration function.The simplified parameter set (removing reserve ratios and treasury revenue target) and fixed treasury fee approach correctly implement the PR objectives. The Mordor configuration appropriately uses the SHU oracle to preserve dual-oracle logic.
157-159: Good fallback handling for unsupported networks.The explicit revert with a descriptive error message prevents silent failures and ensures that only supported networks can be configured.
keshaviscool
left a comment
keshaviscool
left a comment
There was a problem hiding this comment.
Bug fix: Changed deployment parameter functions to use if/else if/else chain with explicit revert for unsupported networks, preventing uninitialized variable returns.
- Add HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET constant (address(0) placeholder) - Change getTefnutConfigFromNetwork to use IOracleShu-compatible oracle on mainnet - Add require() check to prevent deployment until ShuOracleConverter is deployed - Convert independent if statements to if/else if/else chain - Add explanatory comments about intentional mainnet safeguard Addresses CodeRabbit review comment about oracle interface mismatch. DjedTefnut requires IOracleShu (readMaxPrice/readMinPrice) but mainnet HebeSwap oracle only implements IOracle (readPrice). The require() check ensures deployment will revert with clear instructions until maintainers deploy ShuOracleConverter on mainnet.
keshaviscool
left a comment
keshaviscool
left a comment
There was a problem hiding this comment.
Fixed! Changed to use HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET (IOracleShu interface) instead of the IOracle implementation. Added a require() check that will revert deployment until ShuOracleConverter is deployed on mainnet by maintainers. Used address(0) placeholder with TODO comments explaining deployment steps.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/DeploymentParameters.sol (1)
133-200: Consider returning locals (and using custom errors) instead of mutating storage scratch vars.
Right nowgetTefnutConfigFromNetwork()writes tooracleAddress,SCALING_FACTOR, etc. Using locals makes this function easier to reason about and prevents “stale state” surprises if this contract is inherited/used in multiple steps.function getTefnutConfigFromNetwork( SupportedNetworks network ) internal returns ( address, address, uint256, uint256, uint256, uint256, uint256, uint256, uint256 ) { + address _oracleAddress; + address _treasuryAddress; + uint256 _SCALING_FACTOR; + uint256 _INITIAL_TREASURY_FEE; + uint256 _FEE; + uint256 _THREASHOLD_SUPPLY_SC; + uint256 _RESERVE_COIN_MINIMUM_PRICE; + uint256 _RESERVE_COIN_INITIAL_PRICE; + uint256 _TX_LIMIT; + if (network == SupportedNetworks.ETHEREUM_SEPOLIA) { - oracleAddress = CHAINLINK_SEPOLIA_INVERTED_ORACLE_ADDRESS; - treasuryAddress = 0x0f5342B55ABCC0cC78bdB4868375bCA62B6c16eA; - SCALING_FACTOR=1e24; - INITIAL_TREASURY_FEE=25e20; // Used as fixed treasury fee for Tefnut - FEE=15e21; - THREASHOLD_SUPPLY_SC=5e11; - RESERVE_COIN_MINIMUM_PRICE=1e18; - RESERVE_COIN_INITIAL_PRICE=1e20; - TX_LIMIT=1e10; + _oracleAddress = CHAINLINK_SEPOLIA_INVERTED_ORACLE_ADDRESS; + _treasuryAddress = 0x0f5342B55ABCC0cC78bdB4868375bCA62B6c16eA; + _SCALING_FACTOR = 1e24; + _INITIAL_TREASURY_FEE = 25e20; + _FEE = 15e21; + _THREASHOLD_SUPPLY_SC = 5e11; + _RESERVE_COIN_MINIMUM_PRICE = 1e18; + _RESERVE_COIN_INITIAL_PRICE = 1e20; + _TX_LIMIT = 1e10; } else if (network == SupportedNetworks.ETHEREUM_CLASSIC_MORDOR) { - oracleAddress = HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MORDOR; + _oracleAddress = HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MORDOR; ... } else if (network == SupportedNetworks.ETHEREUM_CLASSIC_MAINNET) { require( HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET != address(0), "Deploy ShuOracleConverter on mainnet before using Tefnut. Run: forge script scripts/deployOracleConverter.s.sol" ); - oracleAddress = HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET; + _oracleAddress = HEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET; ... } else { revert(string(abi.encodePacked( "Tefnut not supported on network: ", networks[network] ))); } return ( - oracleAddress, - treasuryAddress, - SCALING_FACTOR, - INITIAL_TREASURY_FEE, // Fixed treasury fee (no decay) - FEE, - THREASHOLD_SUPPLY_SC, - RESERVE_COIN_MINIMUM_PRICE, - RESERVE_COIN_INITIAL_PRICE, - TX_LIMIT + _oracleAddress, + _treasuryAddress, + _SCALING_FACTOR, + _INITIAL_TREASURY_FEE, + _FEE, + _THREASHOLD_SUPPLY_SC, + _RESERVE_COIN_MINIMUM_PRICE, + _RESERVE_COIN_INITIAL_PRICE, + _TX_LIMIT ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/DeploymentParameters.sol(4 hunks)
🔇 Additional comments (3)
scripts/DeploymentParameters.sol (3)
38-49: Mainnet placeholder + deployment notes are a good fail-fast pattern.
Nice: keepingHEBESWAP_SHU_ORACLE_INVERTED_ADDRESS_MAINNET = address(0)forces an explicit maintainer action before mainnet Tefnut deploys.
13-17: Enum extension is type-safe; the actual risk is logic confusion when callers pass the wrong version parameter.The
DJED_TEFNUTenum value is used ingetConfigFromNetwork()(line 88) to select the SHU oracle on Mordor. However,deployDjedTefnutContract.s.soldoesn't usegetConfigFromNetwork()at all—it calls the separategetTefnutConfigFromNetwork()function instead and has no version parameter.The three deployment scripts that do accept a
SupportedVersionparameter (DeployDjed,DeployDjedShu,DeployOracleConverter) are separate contracts, and Foundry's type system enforces named enum usage (not raw ints) when invoked. So the original concern about numeric enum values is not a practical issue here.The subtle hazard is: if someone mistakenly calls
getConfigFromNetwork(network, SupportedVersion.DJED_TEFNUT)when deploying a non-Tefnut contract, it will silently use the SHU oracle logic on Mordor (per line 88), which may not match the contract's requirements. Document the version parameter's intended scope clearly to prevent this misuse.
87-100: Mordor routing for Tefnut is correct, but getConfigFromNetwork() should explicitly reject DJED_TEFNUT on mainnet and sepolia to prevent accidental misuse.The current code correctly routes Tefnut to the SHU oracle on Mordor (line 88), and deployDjedTefnutContract.s.sol properly uses the dedicated getTefnutConfigFromNetwork() function. However, getConfigFromNetwork() accepts a version parameter but ignores it on mainnet and sepolia—meaning if someone calls getConfigFromNetwork(ETHEREUM_CLASSIC_MAINNET, DJED_TEFNUT), it would return the non-SHU oracle. While getTefnutConfigFromNetwork() has a safeguard via require() check, getConfigFromNetwork() should explicitly reject DJED_TEFNUT on networks where it's not supported to prevent accidental oracle misrouting through this function.
…nfigFromNetwork - Add require() check to reject DJED_TEFNUT in getConfigFromNetwork() - Remove DJED_TEFNUT from Mordor oracle routing logic - Add version-based oracle selection for DJED_SHU on mainnet - Convert getConfigFromNetwork to proper if/else if/else chain - Add documentation comments explaining function scope Prevents accidental oracle misrouting when DJED_TEFNUT is passed to getConfigFromNetwork() instead of getTefnutConfigFromNetwork(). Addresses CodeRabbit review feedback about potential misuse scenarios.
keshaviscool
left a comment
keshaviscool
left a comment
There was a problem hiding this comment.
Fixed! Added explicit require() check to reject DJED_TEFNUT in getConfigFromNetwork() and added version-based oracle selection for mainnet. This prevents accidental oracle interface mismatches across protocol variants.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/DeploymentParameters.sol (1)
63-71: Fix return arity mismatch ingetConfigFromNetwork(won’t compile as written).
Thereturns (...)list declares 12uint256values (Line 69-71), but thereturn (...)only returns 10uint256values (Line 143-152). Align the signature with the actual returned fields.function getConfigFromNetwork( SupportedNetworks network, SupportedVersion version ) internal returns ( address, address, - uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256 + uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256, uint256 ) { ... }Also applies to: 140-153
🧹 Nitpick comments (1)
scripts/DeploymentParameters.sol (1)
50-52: Optional: avoid mutating storage for config getters (use locals / structs / custom errors).
Both getters write to storage fields (oracleAddress,SCALING_FACTOR, etc.) before returning. For script-only usage this is fine, but using local variables (and optionally returning a struct + custom errors) avoids stale-state hazards if these functions are ever reused outside scripts.Also applies to: 63-154, 161-223
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/DeploymentParameters.sol(8 hunks)
🔇 Additional comments (2)
scripts/DeploymentParameters.sol (2)
38-49: Verify Sepolia oracle implementsIOracleShufor Tefnut.
getTefnutConfigFromNetwork()usesCHAINLINK_SEPOLIA_INVERTED_ORACLE_ADDRESSfor Sepolia (Line 171). Since Tefnut requires the SHU dual-oracle interface, please double-check this address is a ShuOracleConverter /IOracleShuimplementation (not a plainIOracle), similar to the mainnet safeguard.Also applies to: 156-160, 170-196
60-78: Nice safety improvements (explicit routing + unsupported-network reverts + mainnet guard).
RejectingDJED_TEFNUTingetConfigFromNetwork(Line 74-77), converting to anelse ifchain (Line 93+), and adding explicit reverts (Line 133-138, 205-210) all reduce silent misconfiguration risk. The mainnetaddress(0)guard for the SHU oracle is also a good fail-fast pattern (Line 192-195).Also applies to: 93-138, 156-223
|
@yogesh0509 Actionable comments is 0. please review I have important (deployed address for ShuOracleConverter should be provided by maintainer ) |
|
Thanks, I will review it in some time . |
2 participants
How to Verify This PR
Compile all contracts
forge build
Run Tefnut test suite
forge test --match-contract DjedTefnutTest -vv
Resolves #37
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.