feat: implemented advanced Defi/AMM contracts#22
feat: implemented advanced Defi/AMM contracts#22emarc99 wants to merge 7 commits intotrustbridgecr:mainfrom
Conversation
WalkthroughAdds six new contract crates to the workspace and implements full modules for AMM, Flash Loan Router, Liquidity Mining, Options, Synthetic Asset Factory, and Yield Strategy, including errors, events, storage, and core contract logic. Updates top-level Cargo workspace members to include these contracts. No removals noted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AMM as AMM Contract
participant TokenA as Token A
participant TokenB as Token B
User->>AMM: swap(from, sell_token, sell_amount, min_buy)
AMM->>AMM: validate auth/amount, get reserves, fee
AMM->>TokenA: transfer from User to AMM (sell)
AMM->>AMM: compute buy_amount via x*y=k with fee
AMM->>TokenB: transfer from AMM to User (buy)
AMM->>AMM: update reserves, emit SwapEvent
AMM-->>User: Result(buy_amount)
sequenceDiagram
autonumber
actor Borrower
participant FLR as FlashLoanRouter
participant Pool as Pool (Token)
participant Token as ERC-20 (Soroban Token)
Borrower->>FLR: flash_loan(receiver, token, amount, data)
FLR->>FLR: select pool, compute fee
FLR->>Token: transfer amount to receiver
Note over Borrower,Token: Receiver uses funds, must repay amount+fee within tx
FLR->>Token: verify/collect repayment from receiver
FLR->>FLR: update totals, emit FlashLoanEvent
FLR-->>Borrower: Ok(true)
sequenceDiagram
autonumber
actor User
participant LM as LiquidityMining
participant StakeTok as Staking Token
participant RewardTok as Reward Token(s)
User->>LM: stake(amount)
LM->>LM: update rewards, totals, user stake
LM->>StakeTok: transfer from User to LM
LM->>LM: emit StakeEvent
User-->>LM: Ok(())
User->>LM: claim_rewards()
LM->>LM: compute earned per reward token
LM->>RewardTok: transfer rewards to User
LM->>LM: emit ClaimEvent(s)
User-->>LM: Ok(())
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
Cargo.toml(1 hunks)contracts/amm/Cargo.toml(1 hunks)contracts/amm/src/errors.rs(1 hunks)contracts/amm/src/events.rs(1 hunks)contracts/amm/src/lib.rs(1 hunks)contracts/amm/src/storage.rs(1 hunks)contracts/flash-loan-router/Cargo.toml(1 hunks)contracts/flash-loan-router/src/errors.rs(1 hunks)contracts/flash-loan-router/src/events.rs(1 hunks)contracts/flash-loan-router/src/lib.rs(1 hunks)contracts/flash-loan-router/src/storage.rs(1 hunks)contracts/liquidity-mining/Cargo.toml(1 hunks)contracts/liquidity-mining/src/errors.rs(1 hunks)contracts/liquidity-mining/src/events.rs(1 hunks)contracts/liquidity-mining/src/lib.rs(1 hunks)contracts/liquidity-mining/src/storage.rs(1 hunks)contracts/options/Cargo.toml(1 hunks)contracts/options/src/errors.rs(1 hunks)contracts/options/src/events.rs(1 hunks)contracts/options/src/lib.rs(1 hunks)contracts/options/src/storage.rs(1 hunks)contracts/synthetic-asset-factory/Cargo.toml(1 hunks)contracts/synthetic-asset-factory/src/errors.rs(1 hunks)contracts/synthetic-asset-factory/src/events.rs(1 hunks)contracts/synthetic-asset-factory/src/lib.rs(1 hunks)contracts/synthetic-asset-factory/src/storage.rs(1 hunks)contracts/yield-strategy/Cargo.toml(1 hunks)contracts/yield-strategy/src/errors.rs(1 hunks)contracts/yield-strategy/src/events.rs(1 hunks)contracts/yield-strategy/src/lib.rs(1 hunks)contracts/yield-strategy/src/storage.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
contracts/options/src/storage.rs (1)
contracts/options/src/lib.rs (1)
get_option(400-402)
contracts/yield-strategy/src/storage.rs (1)
contracts/yield-strategy/src/lib.rs (3)
get_strategies(322-324)get_total_assets(297-299)get_user_shares(302-304)
contracts/liquidity-mining/src/lib.rs (1)
contracts/liquidity-mining/src/storage.rs (20)
get_staking_token(19-21)put_staking_token(23-25)get_reward_tokens(28-30)put_reward_tokens(32-34)get_total_staked(37-39)put_total_staked(41-43)get_user_stake(46-51)put_user_stake(53-57)get_reward_rate(60-65)put_reward_rate(67-71)get_last_update_time(74-79)put_last_update_time(81-83)get_reward_per_token_stored(86-91)put_reward_per_token_stored(93-97)get_user_reward_per_token_paid(100-105)put_user_reward_per_token_paid(107-111)get_user_rewards(114-119)put_user_rewards(121-125)get_period_finish(128-133)put_period_finish(135-139)
contracts/flash-loan-router/src/lib.rs (2)
contracts/flash-loan-router/src/storage.rs (10)
get_admin(15-17)put_admin(19-21)get_pools(24-29)put_pools(31-33)get_fee_rate(36-38)put_fee_rate(40-42)get_total_borrowed(45-47)put_total_borrowed(49-51)get_total_fees(54-56)put_total_fees(58-60)contracts/tbrg-token/src/contract.rs (1)
balance(98-103)
contracts/yield-strategy/src/lib.rs (1)
contracts/yield-strategy/src/storage.rs (14)
get_vault_token(27-29)put_vault_token(31-33)get_strategies(36-41)put_strategies(43-45)get_total_assets(48-50)put_total_assets(52-54)get_total_shares(57-59)put_total_shares(61-63)get_user_shares(66-71)put_user_shares(73-77)get_strategy_assets(94-99)put_strategy_assets(101-105)get_admin(18-20)put_admin(22-24)
contracts/amm/src/storage.rs (1)
contracts/amm/src/lib.rs (2)
get_total_shares(327-329)get_fee_rate(342-344)
contracts/synthetic-asset-factory/src/lib.rs (1)
contracts/synthetic-asset-factory/src/storage.rs (18)
get_oracle(18-20)put_oracle(22-24)get_collateral_token(27-29)put_collateral_token(31-33)get_synthetic_asset(36-38)put_synthetic_asset(40-42)get_collateral_ratio(45-47)put_collateral_ratio(49-51)get_user_collateral(63-68)put_user_collateral(70-74)get_user_synthetic(77-82)put_user_synthetic(84-88)get_total_collateral(91-93)put_total_collateral(95-97)get_total_synthetic(100-102)put_total_synthetic(104-106)get_liquidation_ratio(54-56)put_liquidation_ratio(58-60)
contracts/synthetic-asset-factory/src/storage.rs (1)
testing/test-suites/src/asset_fetcher.rs (1)
asset_addresses(71-76)
contracts/options/src/lib.rs (1)
contracts/options/src/storage.rs (6)
get_next_option_id(22-24)put_next_option_id(26-28)get_option(31-36)put_option(38-42)get_oracle(13-15)put_oracle(17-19)
contracts/flash-loan-router/src/storage.rs (1)
contracts/flash-loan-router/src/lib.rs (4)
get_pools(257-259)get_fee_rate(262-264)get_total_borrowed(247-249)get_total_fees(252-254)
contracts/amm/src/lib.rs (1)
contracts/amm/src/storage.rs (14)
get_reserve_a(34-36)get_reserve_b(43-45)get_token_a(16-18)get_token_b(25-27)get_total_shares(52-54)put_reserve_a(38-40)put_reserve_b(47-49)put_token_a(20-22)put_token_b(29-31)put_total_shares(56-58)get_shares(70-75)put_shares(77-81)get_fee_rate(61-63)put_fee_rate(65-67)
contracts/liquidity-mining/src/storage.rs (1)
contracts/liquidity-mining/src/lib.rs (2)
get_total_staked(244-246)get_user_stake(239-241)
🔇 Additional comments (6)
contracts/options/Cargo.toml (1)
1-12: LGTM! Standard Soroban contract configuration.The manifest follows standard patterns for Soroban smart contracts with appropriate cdylib configuration and workspace-managed dependencies.
contracts/yield-strategy/Cargo.toml (1)
1-12: LGTM! Standard Soroban contract configuration.The manifest follows standard patterns for Soroban smart contracts with appropriate cdylib configuration and workspace-managed dependencies.
contracts/flash-loan-router/Cargo.toml (1)
1-12: LGTM! Standard Soroban contract configuration.The manifest follows standard patterns for Soroban smart contracts with appropriate cdylib configuration and workspace-managed dependencies.
contracts/liquidity-mining/Cargo.toml (1)
1-12: LGTM! Standard Soroban contract configuration.The manifest follows standard patterns for Soroban smart contracts with appropriate cdylib configuration and workspace-managed dependencies.
Cargo.toml (1)
6-12: LGTM! Workspace expanded correctly with new DeFi contracts.The additions of six new contract crates (amm, liquidity-mining, synthetic-asset-factory, options, yield-strategy, flash-loan-router) are correctly configured and integrate properly with the existing workspace structure.
contracts/options/src/errors.rs (1)
1-23: LGTM! Comprehensive error coverage for options contract.The error enum provides thorough coverage of the options lifecycle with appropriate variants for parameter validation, state checks, expiry handling, and permission verification. The documentation and structure follow Soroban best practices.
| [package] | ||
| name = "amm" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
| authors = ["TrustBridge Team"] | ||
|
|
||
| [lib] | ||
| crate-type = ["cdylib"] | ||
|
|
||
| [dependencies] | ||
| soroban-sdk = { workspace = true } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify if additional math dependencies are needed for AMM operations.
The manifest looks correct, but AMM contracts typically require advanced math operations (square root calculations, fixed-point arithmetic for pricing). Confirm that soroban-sdk provides all necessary numeric primitives, or consider adding specialized math libraries if needed.
🏁 Script executed:
#!/bin/bash
# Description: Check if soroban-sdk provides sqrt and other math operations needed for AMM
# Search for sqrt usage in AMM implementation
rg -n --type=rust -C3 'sqrt' contracts/amm/
# Search for math operations or numeric types
rg -n --type=rust -C3 '(U256|I256|fixed.*point|FixedPoint)' contracts/amm/Length of output: 0
| pub fn initialize( | ||
| e: Env, | ||
| token_a: Address, | ||
| token_b: Address, | ||
| fee_rate: u32, | ||
| ) -> Result<(), AmmError> { | ||
| if token_a >= token_b { | ||
| return Err(AmmError::InvalidTokenOrder); | ||
| } | ||
|
|
||
| if fee_rate > 10000 { | ||
| return Err(AmmError::InvalidFeeRate); | ||
| } | ||
|
|
||
| put_token_a(&e, token_a); | ||
| put_token_b(&e, token_b); | ||
| put_fee_rate(&e, fee_rate); | ||
| put_total_shares(&e, 0); | ||
| put_reserve_a(&e, 0); | ||
| put_reserve_b(&e, 0); | ||
| Ok(()) |
There was a problem hiding this comment.
Lock down initialization
initialize can be called repeatedly by any address. After the pool holds funds, a malicious caller could invoke it again, overwrite the stored token pair/reserves/shares back to zero, and permanently brick existing liquidity. Add a one-time guard (e.g., check a stored sentinel or nonzero reserves/shares) or restrict the call to an authorized admin before writing state.
🤖 Prompt for AI Agents
In contracts/amm/src/lib.rs around lines 32 to 52, initialize currently allows
anyone to call it repeatedly and will overwrite token addresses, fee_rate,
reserves and shares; add a one-time guard to prevent re-initialization once the
contract is set up: check a stored sentinel or that total_shares or reserves are
nonzero (or store an initialized boolean) at the start and return
Err(AmmError::AlreadyInitialized) if set; alternatively restrict access by
checking the caller against a stored admin/owner address and only allow the
first caller to perform initialization; persist the chosen sentinel/admin before
writing other state and keep all existing validation (token order and fee rate)
intact.
| admin.require_auth(); | ||
|
|
||
| if fee_rate > 1000 { | ||
| return Err(FlashLoanError::InvalidFeeRate); | ||
| } | ||
|
|
||
| put_admin(&e, admin); | ||
| put_fee_rate(&e, fee_rate); | ||
| put_total_borrowed(&e, 0); | ||
| put_total_fees(&e, 0); | ||
|
|
||
| let pools = Vec::new(&e); | ||
| put_pools(&e, pools); | ||
| Ok(()) |
There was a problem hiding this comment.
Initialization is vulnerable to hostile reconfiguration
initialize can be invoked repeatedly by any caller who signs as the admin argument. There is no guard that the router was already initialized nor that the stored admin matches the caller. An attacker can therefore call initialize after deployment, overwrite all critical state (admin, fee, pools, totals), and seize control of the router. Persistently stored pools would also be wiped. Please add an initialization flag (or check instance().has(&DataKey::Admin)) and reject re-entry with a dedicated error before writing state.
🤖 Prompt for AI Agents
In contracts/flash-loan-router/src/lib.rs around lines 47 to 60, the initialize
function currently allows repeated reconfiguration by any caller who signs as
the provided admin; add a guard that rejects re-entry: before writing state
check persistent storage for an existing admin (e.g.,
instance().has(&DataKey::Admin) or get_admin(&e).is_some()), and if present
return a new dedicated error like FlashLoanError::AlreadyInitialized; only
proceed to require_auth and write admin/fee/pools/totals when no admin exists,
so initialization can only happen once and cannot overwrite stored state.
| let fee = amount | ||
| .checked_mul(get_fee_rate(&e) as i128).unwrap() | ||
| .checked_div(10000).unwrap(); | ||
|
|
There was a problem hiding this comment.
Avoid panics on fee arithmetic overflow
The fee calculation unwraps checked_mul/checked_div. For sufficiently large amounts (close to i128::MAX) these calls return None, triggering a panic and aborting the transaction. Please propagate an explicit error (e.g., FlashLoanError::ArithmeticOverflow) instead of unwrapping.
Also applies to: 241-243
🤖 Prompt for AI Agents
In contracts/flash-loan-router/src/lib.rs around lines 153-156 (and similarly at
241-243), the fee calculation uses unwraps on checked_mul/checked_div which can
panic; replace those unwraps with error propagation so the function returns a
FlashLoanError::ArithmeticOverflow on overflow (e.g., use
checked_mul(...).and_then(|v|
v.checked_div(...)).ok_or(FlashLoanError::ArithmeticOverflow)? or chain ok_or
after each checked_* and propagate with ?), ensuring the function signature
returns Result and the error variant is used consistently.
| let token_client = soroban_sdk::token::TokenClient::new(&e, &token); | ||
| token_client.transfer(&pool_addr, &receiver, &amount); | ||
|
|
||
| // Call receiver's flash loan callback | ||
| // In production, this would invoke the receiver contract's callback function | ||
| // For this implementation, we assume the receiver handles the loan logic | ||
|
|
||
| // Verify repayment | ||
| let balance_after = token_client.balance(&e.current_contract_address()); | ||
|
|
||
| // Transfer repayment from receiver to pool | ||
| token_client.transfer(&receiver, &pool_addr, &total_repayment); | ||
|
|
||
| // Update statistics |
There was a problem hiding this comment.
Flash loan transfers cannot succeed and repayment is unenforceable
token_client.transfer(&pool_addr, &receiver, &amount) requires the pool address to authorize the spend. Because this router contract is the caller, and the pool contract/account never calls require_auth, the transfer will revert. Likewise, the later transfer that pulls repayment from the receiver has the same problem. Beyond that, no callback is issued to the receiver so it cannot execute its loan logic or approve repayment. The flash loan flow is therefore non-functional. You need to either (a) have the router custody liquidity itself, moving tokens it controls, or (b) invoke pool / receiver callbacks that in turn authorize the transfers, adhering to ERC‑3156 semantics (borrow → receiver callback → receiver repays router → router returns funds). As written, every flash loan attempt will fail.
| put_user_synthetic(&e, &user, user_synthetic - synthetic_amount); | ||
| put_user_collateral(&e, &user, user_collateral - actual_collateral_seized); | ||
| put_total_synthetic(&e, get_total_synthetic(&e) - synthetic_amount); | ||
| put_total_collateral(&e, get_total_collateral(&e) - actual_collateral_seized); |
There was a problem hiding this comment.
Apply checked arithmetic in liquidation bookkeeping
These updates subtract directly from totals and per-user balances. In release builds an underflow wraps instead of reverting, so a liquidator can push totals negative and desynchronize accounting. Use checked_sub/ok_or (and checked_add where needed) to keep invariants intact.
🤖 Prompt for AI Agents
In contracts/synthetic-asset-factory/src/lib.rs around lines 266 to 269, the
code performs unchecked subtraction on per-user and global balances which can
underflow in release builds; replace these direct `-` operations with
`checked_sub(...).ok_or(...)` (and use `checked_add`/`ok_or` for any increments)
and propagate the error (or return a descriptive ContractError) so the function
returns a Result and rejects operations that would underflow or overflow,
updating callers/signature as needed to handle the error.
| /// Internal: Get collateral value (placeholder - should query oracle) | ||
| fn get_collateral_value(_e: &Env, amount: i128) -> i128 { | ||
| // In production, this would query the oracle for the collateral price | ||
| // For now, assuming 1:1 value | ||
| amount | ||
| } | ||
|
|
||
| /// Internal: Get synthetic value (placeholder - should query oracle) | ||
| fn get_synthetic_value(_e: &Env, amount: i128) -> i128 { | ||
| // In production, this would query the oracle for the synthetic asset price | ||
| // For now, assuming 1:1 value | ||
| amount | ||
| } |
There was a problem hiding this comment.
Collateral/Synth valuation ignores the oracle
Both value helpers just echo the raw amount, so every token is treated 1:1 regardless of price. Collateral ratio checks therefore never reflect market reality; a 90%-crashed collateral still reads “healthy”. You already persist the oracle address—wire it in here (or plumb a price feed client) so the health checks and liquidation math use oracle prices.
| /// Initialize the yield strategy manager | ||
| /// | ||
| /// # Arguments | ||
| /// * `admin` - Administrator address | ||
| /// * `vault_token` - Token to be managed in the vault | ||
| pub fn initialize(e: Env, admin: Address, vault_token: Address) -> Result<(), YieldStrategyError> { | ||
| admin.require_auth(); | ||
|
|
||
| put_admin(&e, admin); | ||
| put_vault_token(&e, vault_token); | ||
| put_total_assets(&e, 0); | ||
| put_total_shares(&e, 0); | ||
|
|
||
| let strategies = Vec::new(&e); | ||
| put_strategies(&e, strategies); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Prevent re-initialization
initialize has no guard. Anyone can call it again after deployment, overwrite the stored admin/vault token, and zero out assets and shares, effectively hijacking the vault. Persist an “initialized” flag (or similar sentinel) and reject subsequent calls.
🤖 Prompt for AI Agents
In contracts/yield-strategy/src/lib.rs around lines 35–51, add a persistent
"initialized" sentinel check at the start of initialize and reject calls if
already set: read a stored initialized flag (or check stored admin presence) and
return a YieldStrategyError like AlreadyInitialized when true; only when not
initialized proceed to require_auth, write
admin/vault_token/total_assets/total_shares/strategies and then set the
initialized flag to true (persist it) before returning Ok.
| fn deposit_to_strategy(e: &Env, strategy: &Address, amount: i128) { | ||
| let vault_token = get_vault_token(e); | ||
| let token_client = soroban_sdk::token::TokenClient::new(e, &vault_token); | ||
| token_client.transfer(&e.current_contract_address(), strategy, &amount); | ||
| } | ||
|
|
||
| /// Internal: Withdraw from strategy | ||
| fn withdraw_from_strategy(e: &Env, strategy: &Address, amount: i128) { | ||
| // In production, this would call the strategy's withdraw function | ||
| // For now, we assume the strategy transfers tokens back | ||
| let vault_token = get_vault_token(e); | ||
| let token_client = soroban_sdk::token::TokenClient::new(e, &vault_token); | ||
| token_client.transfer(strategy, &e.current_contract_address(), &amount); | ||
| } |
There was a problem hiding this comment.
Strategy withdrawals can’t succeed
token_client.transfer(strategy, &e.current_contract_address(), &amount) requires the strategy address to authorize the transfer, but this contract provides no such auth. In practice every withdrawal/rebalance that needs strategy funds will revert. Replace this with an explicit call into each strategy to pull funds back (or another mechanism that causes the strategy to auth the transfer) before updating balances.
🤖 Prompt for AI Agents
In contracts/yield-strategy/src/lib.rs around lines 334 to 347, the
withdraw_from_strategy implementation attempts token_client.transfer(strategy,
&e.current_contract_address(), &amount) which requires the strategy to authorize
the transfer and therefore always fails; replace this with an explicit
cross-contract invocation to the strategy (e.g., call its withdraw/pull
function) that instructs the strategy to transfer the specified amount back to
the vault or otherwise authorizes the token transfer, handle and check the call
result (revert on failure), and only after a successful strategy callback
perform any local balance updates; ensure the call uses the correct function
name/signature expected by deployed strategies and passes the vault address and
amount.
| /// Internal: Withdraw from strategies (priority-based) | ||
| fn withdraw_from_strategies(e: &Env, amount: i128) { | ||
| let mut remaining = amount; | ||
| let strategies = get_strategies(e); | ||
|
|
||
| // Withdraw from strategies in reverse order (last added first) | ||
| for strategy in strategies.iter().rev() { | ||
| if remaining <= 0 { | ||
| break; | ||
| } | ||
|
|
||
| let withdraw_amount = remaining.min(strategy.current_assets); | ||
| if withdraw_amount > 0 { | ||
| Self::withdraw_from_strategy(e, &strategy.address, withdraw_amount); | ||
| remaining -= withdraw_amount; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Keep strategy balances in sync
withdraw_from_strategies updates neither Strategy.current_assets nor the strategy_assets storage helpers, so accounting stays inflated after funds are pulled back. Future allocations/rebalances will operate on stale numbers. Subtract the withdrawn amount from the affected strategy’s stored balance.
🤖 Prompt for AI Agents
In contracts/yield-strategy/src/lib.rs around lines 349 to 366,
withdraw_from_strategies currently calls Self::withdraw_from_strategy but never
updates the stored Strategy.current_assets or the strategy_assets storage
helpers; after each successful withdraw, subtract withdraw_amount from the
strategy’s stored current_assets and persist that change (e.g., load mutable
strategies or iterate by index, update the Strategy record and call the
appropriate storage setter), and also update the strategy_assets storage helper
to reflect the withdrawn amount so on-chain accounting remains correct.
1 participant
Pull Request for TrustBridge - Close Issue
Implements an advanced DeFi protocol with 6 contracts.
Add here some information
🌀 Summary of Changes
🛠 Testing
Evidence Before Solution
Evidence After Solution
✅ ESLint Compliance (Mandatory)
To ensure that the code follows project standards, please run the following command and attach a screenshot of the output:
You should see:
📸 Attach a screenshot showing the result of the lint check:
📂 Related Issue
🎉 Thank you for reviewing this PR! 🎉
Summary by CodeRabbit