Nodemon watches dist directory #302
Closed
githoboman wants to merge 5 commits into
Hidden character warning
The head ref may contain hidden characters: "Nodemon-watches-dist/-directory-\u2013-T"
Closed
Conversation
…minting, reserve, and savings modules
|
Hey @githoboman! 👋 It looks like this PR isn't linked to any issue. If this PR is for one of the issues assigned to you as part of a Wave, please link it to ensure your contribution is tracked properly. You can do this by adding a keyword to the PR description (e.g.,
|
📝 WalkthroughWalkthroughThis PR introduces a two-step admin rotation mechanism with a configurable timelock across five ACBU smart contracts. The feature allows the current admin to nominate a successor with a delay period before the successor can claim ownership, includes cancellation capabilities, and exposes read accessors. Additionally, the savings_vault contract gains staged upgrade logic, and the escrow contract refactors its admin loading for consistency. ChangesTwo-step Admin Rotation and Contract Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
githoboman
changed the title
githoboman
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (14)
acbu_escrow/src/lib.rs (2)
199-199:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winType mismatch:
get_admintakesEnvby value, not by reference.
release()callsSelf::get_admin(&env)butget_admin(env: Env)takesEnvby value. This should useload_admin(&env)for consistency with other admin-gated methods in this file.🐛 Proposed fix
- let admin = Self::get_admin(&env)?; + let admin = Self::load_admin(&env)?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_escrow/src/lib.rs` at line 199, The call in release() uses Self::get_admin(&env) but get_admin expects Env by value, causing a type mismatch; replace that call with Self::load_admin(&env) (which accepts &Env) to match other admin-gated methods and fix the signature mismatch involving Env and functions get_admin and load_admin.
8-24:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing error variants will cause compilation failure.
The admin rotation methods reference error variants that are not defined in
EscrowError:
- Line 334:
EscrowError::NoPendingAdmin- Line 343:
EscrowError::AdminTimelockNotElapsed- Line 368:
EscrowError::NoPendingAdminToCancel🐛 Proposed fix: Add missing error variants
pub enum EscrowError { Paused = 3001, InvalidAmount = 3002, EscrowNotFound = 3003, PayerMismatch = 3004, EscrowExists = 3005, UninitializedAdmin = 3006, UninitializedAcBuToken = 3007, AlreadyInitialized = 3008, TimelockNotElapsed = 3009, NoPendingUpgrade = 3010, Unauthorized = 3011, + NoPendingAdmin = 3012, + AdminTimelockNotElapsed = 3013, + NoPendingAdminToCancel = 3014, Unknown = 3999, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_escrow/src/lib.rs` around lines 8 - 24, The EscrowError enum is missing variants referenced elsewhere (EscrowError::NoPendingAdmin, EscrowError::AdminTimelockNotElapsed, EscrowError::NoPendingAdminToCancel); add these three variants to the EscrowError enum (e.g., assign unique u32 discriminants like 3012, 3013, 3014) so the admin rotation methods compile, keep the #[repr(u32)] and derive attributes intact, and ensure the new names exactly match the symbols used in the codebase.acbu_savings_vault/src/lib.rs (5)
1-8:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate import blocks will cause compilation failure.
Lines 1-2 and 5-8 both import from
soroban_sdk. The imports overlap and will cause duplicate identifier errors.🐛 Proposed fix: Consolidate imports
#![no_std] -use soroban_sdk::{contract, contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env, Vec, Symbol}; - -// --- Definitions (These were missing, now included here) --- -use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env, - Symbol, Vec, -}; +use soroban_sdk::{ + contract, contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env, + Symbol, Vec, +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_savings_vault/src/lib.rs` around lines 1 - 8, There are duplicate import blocks from soroban_sdk (the two separate use lines importing contract, contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env, Symbol, Vec) which will cause compilation errors; fix by consolidating them into a single use soroban_sdk::{...} statement that includes each required symbol (e.g., contract, contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env, Symbol, Vec) and remove the redundant import block so only one unified import exists in lib.rs.
61-70:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
DATA_KEYconst missingpending_adminandpending_admin_eligible_atvalues.The
DataKeystruct (lines 57-58) definespending_adminandpending_admin_eligible_atfields, but theDATA_KEYconst initialization doesn't include values for these fields.🐛 Proposed fix
pub const DATA_KEY: DataKey = DataKey { admin: symbol_short!("admin"), acbu_token: symbol_short!("token"), fee_rate: symbol_short!("fee"), yield_rate: symbol_short!("yield"), paused: symbol_short!("paused"), pending_upgrade_wasm: symbol_short!("upg_wasm"), pending_upgrade_version: symbol_short!("upg_ver"), pending_upgrade_eligible_at: symbol_short!("upg_time"), + pending_admin: symbol_short!("pnd_adm"), + pending_admin_eligible_at: symbol_short!("pnd_eta"), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_savings_vault/src/lib.rs` around lines 61 - 70, DATA_KEY is missing initializers for the DataKey fields pending_admin and pending_admin_eligible_at; update the DATA_KEY const to include values for pending_admin and pending_admin_eligible_at (use symbol_short! or the same convention as other keys, e.g., symbol_short!("pending_admin") and symbol_short!("pending_admin_time") or similar meaningful short symbols) so the DataKey struct initialization in DATA_KEY matches the struct definition (refer to DataKey, DATA_KEY, pending_admin, pending_admin_eligible_at, and symbol_short!).
22-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing error variant
NoPendingAdminToCancel.Line 424 references
Error::NoPendingAdminToCancelwhich is not defined in theErrorenum. However, per the fix above,cancel_upgradeshould not be handling admin cancellation - it should handle upgrade cancellation instead.If admin rotation is intentionally being added to this contract (per the AI summary), the contract needs separate
cancel_admin_transferandcancel_upgradefunctions with appropriate error variants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_savings_vault/src/lib.rs` around lines 22 - 42, The Error enum is missing a variant referenced elsewhere (Error::NoPendingAdminToCancel) and cancel_upgrade was incorrectly handling admin cancellation; add distinct error variants and functions: add Error::NoPendingAdminToCancel (or rename to Error::NoPendingUpgradeToCancel if intent is upgrade cancellation) to the Error enum and split logic so cancel_upgrade only checks and returns the upgrade-related error (e.g., Error::NoPendingUpgradeToCancel), while admin rotation code lives in a separate cancel_admin_transfer function that returns Error::NoPendingAdminToCancel when appropriate; update callers of cancel_upgrade and any match arms to use the correct variant names (Error, cancel_upgrade, cancel_admin_transfer).
72-91:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate
SharedDataKeyandErrordefinitions will cause compilation conflicts.
- Line 73:
SharedDataKeyenum shadows the importedSharedDataKeyfrom thesharedcrate (line 10)- Lines 79-91:
Errorenum is defined again after being defined at lines 22-42These duplicates will cause compilation errors or shadowing issues.
🐛 Proposed fix: Remove duplicate definitions
-#[contracttype] -pub enum SharedDataKey { Version } - #[contracttype] #[derive(Clone)] pub struct DepositLot { pub amount: i128 } -#[contracterror] -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -#[repr(u32)] -pub enum Error { - AlreadyInitialized = 1, - NoAdmin = 2, - NotInitialized = 3, - NoFeeRate = 4, - NoYieldRate = 5, - InvalidVersion = 6, - NoPendingUpgrade = 7, - TimelockNotElapsed = 8, -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_savings_vault/src/lib.rs` around lines 72 - 91, Remove the duplicate local definitions that conflict with imports: delete the local SharedDataKey enum and the duplicate Error enum in this file and rely on the already-imported shared::SharedDataKey and the existing Error definition (the original one at lines 22-42) instead; update any references in this module to use the imported SharedDataKey and the single Error type (and keep DepositLot if it is unique) so there are no shadowing or duplicate-type compilation errors.
415-446:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
load_admindefined twice andcancel_upgradeis corrupted.
load_adminis defined at line 415-416 (incomplete) and again at lines 444-446cancel_upgrade(lines 417-433) appears to be insideload_admin's block and incorrectly implements admin cancellation logic instead of upgrade cancellationThe
cancel_upgradefunction should remove pending upgrade keys, not pending admin keys.🐛 Proposed fix: Separate and correct the functions
fn load_admin(env: &Env) -> Result<Address, Error> { env.storage().instance().get(&DATA_KEY.admin).ok_or(Error::NoAdmin) + } + pub fn cancel_upgrade(env: Env) { let admin = Self::load_admin(&env).unwrap_or_else(|e| env.panic_with_error(e)); admin.require_auth(); - let pending_admin: Address = env - .storage() - .instance() - .get(&DATA_KEY.pending_admin) - .unwrap_or_else(|| env.panic_with_error(Error::NoPendingAdminToCancel)); - env.storage().instance().remove(&DATA_KEY.pending_admin); + env.storage().instance().remove(&DATA_KEY.pending_upgrade_wasm); + env.storage().instance().remove(&DATA_KEY.pending_upgrade_version); - env.storage() - .instance() - .remove(&DATA_KEY.pending_admin_eligible_at); - env.events().publish( - (symbol_short!("adm_cncl"),), - (admin, pending_admin, env.ledger().timestamp()), - ); + env.storage().instance().remove(&DATA_KEY.pending_upgrade_eligible_at); } - - fn load_admin(env: &Env) -> Result<Address, Error> { - env.storage().instance().get(&DATA_KEY.admin).ok_or(Error::NoAdmin) - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_savings_vault/src/lib.rs` around lines 415 - 446, The file has a duplicated load_admin definition and a mangled cancel_upgrade placed inside the first load_admin; fix by removing the duplicate/partial load_admin and restoring a single fn load_admin(&Env) -> Result<Address, Error> that only reads DATA_KEY.admin and returns Ok or Error::NoAdmin, then implement pub fn cancel_upgrade(env: Env) as its own function that: calls Self::load_admin(&env).unwrap_or_else(|e| env.panic_with_error(e)), requires admin.require_auth(), removes DATA_KEY.pending_upgrade and DATA_KEY.pending_upgrade_eligible_at (not pending_admin keys), and publishes the cancellation event (use the existing event pattern and include admin, pending_upgrade address/value, and timestamp). Ensure you reference and use the symbols load_admin, cancel_upgrade, DATA_KEY.pending_upgrade, and DATA_KEY.pending_upgrade_eligible_at.acbu_minting/src/lib.rs (3)
36-54:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
DataKeystruct is missingpending_adminandpending_admin_eligible_atfields.The admin rotation methods (lines 1088-1093, 1120-1123, 1140-1143) reference
DATA_KEY.pending_adminandDATA_KEY.pending_admin_eligible_at, but these fields are not defined in theDataKeystruct.🐛 Proposed fix: Add missing fields
pub struct DataKey { pub admin: Symbol, pub oracle: Symbol, pub reserve_tracker: Symbol, pub acbu_token: Symbol, pub usdc_token: Symbol, pub vault: Symbol, pub treasury: Symbol, pub fee_rate: Symbol, pub fee_single: Symbol, pub paused: Symbol, pub min_mint_amount: Symbol, pub max_mint_amount: Symbol, pub total_supply: Symbol, pub operator: Symbol, pub used_proofs: Symbol, pub processed_fintech_tx_ids: Symbol, pub max_supply: Symbol, + pub pending_admin: Symbol, + pub pending_admin_eligible_at: Symbol, }And add the corresponding values to
DATA_KEYconst:const DATA_KEY: DataKey = DataKey { // ... existing fields ... max_supply: symbol_short!("MAX_SUP"), + pending_admin: symbol_short!("PEND_ADM"), + pending_admin_eligible_at: symbol_short!("PEND_ETA"), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_minting/src/lib.rs` around lines 36 - 54, DataKey is missing the pending_admin and pending_admin_eligible_at symbols referenced elsewhere; add two new pub fields to the DataKey struct named pending_admin and pending_admin_eligible_at (types matching other keys, e.g., Symbol) and then update the DATA_KEY const to include corresponding entries for DATA_KEY.pending_admin and DATA_KEY.pending_admin_eligible_at so the admin rotation methods can access them.
84-107:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing error variants will cause compilation failure.
The contract references error variants not defined in
MintingError:
- Line 973:
MintingError::MaxSupplyExceeded- Line 1106:
MintingError::NoPendingAdmin- Line 1115:
MintingError::AdminTimelockNotElapsed- Line 1139:
MintingError::NoPendingAdminToCancel🐛 Proposed fix: Add missing error variants
pub enum MintingError { // ... existing variants ... InvalidVersion = 5018, + MaxSupplyExceeded = 5019, + NoPendingAdmin = 5020, + AdminTimelockNotElapsed = 5021, + NoPendingAdminToCancel = 5022, Unknown = 5999, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_minting/src/lib.rs` around lines 84 - 107, The MintingError enum is missing variants referenced elsewhere (MaxSupplyExceeded, NoPendingAdmin, AdminTimelockNotElapsed, NoPendingAdminToCancel) which breaks compilation; add these variants to the MintingError enum (keeping #[repr(u32)] and non-duplicating discriminant values—e.g., next unused IDs after InvalidVersion like 5019–5022 or similar) so references like MintingError::MaxSupplyExceeded, MintingError::NoPendingAdmin, MintingError::AdminTimelockNotElapsed, and MintingError::NoPendingAdminToCancel resolve; ensure the new names exactly match usages and update any documentation/comments if present.
1-3:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winOrphan code before
#![no_std]will cause compilation failure.Lines 1-2 contain code before the
#![no_std]attribute on line 3. The#![no_std]attribute must be at the top of the file, and the function stub on line 2 is syntactically invalid.🐛 Proposed fix: Remove orphan code
-use soroban_sdk::{Address, Env, String}; -fn generate_unique_tx_id(env: &Env, _user: &Address, _amount: i128, prefix: &str) -> String { String::from_str(env, prefix) } #![no_std] use soroban_sdk::xdr::ToXdr;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_minting/src/lib.rs` around lines 1 - 3, Move the crate attribute to the very top of the file (place #![no_std] as the first line) and remove or fix the orphan, syntactically-invalid stub before it: delete the stray "use soroban_sdk::{Address, Env, String}; fn generate_unique_tx_id(...)" lines or relocate them after the attribute and implement generate_unique_tx_id properly (keep the function name generate_unique_tx_id and ensure it returns a soroban_sdk::String using String::from_str(env, prefix) with a correct signature and parameter usage).acbu_reserve_tracker/src/lib.rs (1)
16-23:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing error variants will cause compilation failure.
The contract references error variants not defined in
ReserveTrackerError:
- Line 107:
ReserveTrackerError::ZeroSupply- Line 288:
ReserveTrackerError::NoPendingAdmin- Line 297:
ReserveTrackerError::AdminTimelockNotElapsed- Line 320:
ReserveTrackerError::NoPendingAdminToCancel🐛 Proposed fix: Add missing error variants
pub enum ReserveTrackerError { AlreadyInitialized = 8001, InvalidVersion = 8002, + ZeroSupply = 8003, + NoPendingAdmin = 8004, + AdminTimelockNotElapsed = 8005, + NoPendingAdminToCancel = 8006, Unknown = 8999, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_reserve_tracker/src/lib.rs` around lines 16 - 23, The ReserveTrackerError enum is missing variants referenced elsewhere (ZeroSupply, NoPendingAdmin, AdminTimelockNotElapsed, NoPendingAdminToCancel); add these four variants to the ReserveTrackerError enum so references like ReserveTrackerError::ZeroSupply, ::NoPendingAdmin, ::AdminTimelockNotElapsed and ::NoPendingAdminToCancel compile, and assign each a unique u32 discriminant consistent with the existing values (e.g., continuing the 8001/8002/8999 pattern) to avoid collisions; update the #[repr(u32)] enum ReserveTrackerError declaration accordingly.acbu_lending_pool/src/lib.rs (1)
99-116:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing error variants will cause compilation failure.
The admin rotation methods reference error variants not defined in the
Errorenum:
- Line 585:
Error::NoPendingAdmin- Line 594:
Error::AdminTimelockNotElapsed- Line 617:
Error::NoPendingAdminToCancel🐛 Proposed fix: Add missing error variants
pub enum Error { NotFound = 1, InvalidState = 2, Unauthorized = 3, AlreadyInitialized = 4, InvalidAmount = 5, InsufficientBalance = 6, InsufficientCollateral = 7, InsufficientLiquidity = 8, Paused = 2001, InvalidVersion = 2002, TimelockNotElapsed = 2003, NoPendingUpgrade = 2004, + NoPendingAdmin = 2005, + AdminTimelockNotElapsed = 2006, + NoPendingAdminToCancel = 2007, Unknown = 2999, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_lending_pool/src/lib.rs` around lines 99 - 116, The Error enum is missing variants referenced by the admin-rotation logic (Error::NoPendingAdmin, Error::AdminTimelockNotElapsed, Error::NoPendingAdminToCancel), causing compile errors; open the enum definition (pub enum Error) and add these three variants with unique repr(u32) discriminants (e.g. next unused values such as 2005, 2006, 2007 or similar) so they do not collide with existing codes, preserving #[contracterror] and the derive attributes and keeping the enum ordering/format consistent.acbu_burning/src/lib.rs (2)
1-21:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate import blocks will cause compilation failure.
Lines 1-10 and 12-21 both declare
use soroban_sdk::{...}blocks. The second block redeclarescontract,contractimpl,contracttype,symbol_short,vec,Address,Env,IntoVal,Symbol,Vecwhich will cause a compilation error.Additionally, line 91 references
CONTRACT_VERSIONwhich was removed from thesharedimports but is still used ininitialize.🐛 Proposed fix: Remove duplicate imports and restore CONTRACT_VERSION
use soroban_sdk::{ - contract, contractimpl, contracttype, symbol_short, vec, Address, Env, - IntoVal, Symbol, Vec, -}; - -use shared::{ - calculate_fee, ContractError, CurrencyCode, DataKey as SharedDataKey, BASIS_POINTS, - DECIMALS, MIN_BURN_AMOUNT, ORACLE_GET_CURRENCIES, ORACLE_GET_BASKET_WEIGHT, - ORACLE_GET_RATE, ORACLE_GET_S_TOKEN_ADDR, -}; - - contract, contractimpl, contracttype, symbol_short, vec, Address, BytesN, Env, IntoVal, + contract, contractimpl, contracttype, symbol_short, vec, Address, BytesN, Env, IntoVal, String as SorobanString, Symbol, Vec, }; use shared::{ - calculate_fee, BurnEvent, ContractError, CurrencyCode, DataKey as SharedDataKey, BASIS_POINTS, + calculate_fee, BurnEvent, ContractError, CONTRACT_VERSION, CurrencyCode, DataKey as SharedDataKey, BASIS_POINTS, DECIMALS, MIN_BURN_AMOUNT, ORACLE_GET_ACBU_RATE, ORACLE_GET_CURRENCIES, ORACLE_GET_BASKET_WEIGHT, ORACLE_GET_RATE, ORACLE_GET_S_TOKEN_ADDR, UPDATE_INTERVAL_SECONDS, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_burning/src/lib.rs` around lines 1 - 21, Remove the duplicated soroban_sdk import block (the second use ... block that re-declares contract, contractimpl, contracttype, symbol_short, vec, Address, Env, IntoVal, Symbol, Vec/BytesN, SorobanString) so there is only a single consolidated soroban_sdk import; also restore CONTRACT_VERSION in the shared import list so the symbol used in initialize (CONTRACT_VERSION) is available (add CONTRACT_VERSION back to the use shared::{...} import alongside the other shared symbols).
323-383:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate code blocks in
redeem_basketwill cause double transfers and compilation issues.Several code blocks are duplicated:
- Line 323:
currenciesis fetched again after line 313-317- Lines 328-332:
recipient,weight,rate,stokenare fetched inline, then again at lines 339-364- Lines 380-383:
transfer_fromis called twice for the samenative_iamount - this will transfer double the intended amount or fail on insufficient balanceThis appears to be a merge conflict or copy-paste error that will cause incorrect financial transfers.
🐛 Proposed fix: Remove duplicate code blocks
let currencies: Vec<CurrencyCode> = env.invoke_contract( &oracle_addr, &Symbol::new(&env, ORACLE_GET_CURRENCIES), vec![&env], ); if currencies.is_empty() { env.panic_with_error(ContractError::InvalidCurrency); } - let currencies: Vec<CurrencyCode> = env.invoke_contract(&oracle_addr, &Symbol::new(&env, ORACLE_GET_CURRENCIES), vec![&env]); - let mut amounts_out = Vec::new(&env); for i in 0..currencies.len() { let currency = currencies.get(i).unwrap(); - let recipient = recipients.get(i).unwrap(); - - let weight: i128 = env.invoke_contract(&oracle_addr, &Symbol::new(&env, ORACLE_GET_BASKET_WEIGHT), vec![&env, currency.clone().into_val(&env)]); - let rate: i128 = env.invoke_contract(&oracle_addr, &Symbol::new(&env, ORACLE_GET_RATE), vec![&env, currency.clone().into_val(&env)]); - let stoken: Address = env.invoke_contract(&oracle_addr, &Symbol::new(&env, ORACLE_GET_S_TOKEN_ADDR), vec![&env, currency.clone().into_val(&env)]); - // C-057: Each currency slot maps to the corresponding recipient by index. if i >= recipients.len() {And remove the duplicate transfer:
if native_i > 0 { let token = soroban_sdk::token::Client::new(&env, &stoken); - token.transfer_from(&env.current_contract_address(), &vault, &recipient, &native_i); let spender = env.current_contract_address(); token.transfer_from(&spender, &vault, &recipient, &native_i); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@acbu_burning/src/lib.rs` around lines 323 - 383, In redeem_basket: duplicate blocks caused double lookups and double transfers—remove the repeated retrievals and duplicated logic so each currency loop iteration only fetches currency, recipient, weight, rate, and stoken once (use the first set of calls to CurrencyCode list, recipients.get(i), env.invoke_contract calls for ORACLE_GET_BASKET_WEIGHT, ORACLE_GET_RATE, ORACLE_GET_S_TOKEN_ADDR) and eliminate the second duplicated block including the extra token.transfer_from; also move the recipient bounds check (i >= recipients.len()) before any recipients.get(i) and keep only one token.transfer_from call for native_i (or one transfer_from pair if two different spender/vault semantics are required but not duplicated). Ensure variables used later (acbu_gross_i, fee_i, native_i) come from the single non-duplicated computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@acbu_savings_vault/src/lib.rs`:
- Around line 359-381: The pause() function body currently contains upgrade
proposal logic; replace that body so pause() simply sets the contract paused
flag (e.g., set &DATA_KEY.paused or &SharedDataKey::Paused to true) and performs
admin.require_auth() as needed, removing any references to
new_version/new_wasm_hash/UPGRADE_TIMELOCK_SECONDS; also delete the first
duplicate execute_upgrade function (the one that updates pending_upgrade_*
storage and calls update_current_contract_wasm) and keep the correct
execute_upgrade implementation only once.
In `@nodemon.json`:
- Around line 1-6: The PR title is inaccurate: nodemon.json's "watch" key is set
to "src" and "ignore" includes "dist", so update the PR title to reflect that
nodemon watches the src directory (not dist); search for the "watch" and
"ignore" entries in nodemon.json to confirm and then change the PR title to
something like "Nodemon watches src/ directory" or an equivalent accurate
description.
- Line 5: The nodemon config's exec value "ts-node src/index.ts" points to a
non-existent entrypoint and will fail startup; update nodemon.json to either set
exec to the real runtime command for this project (replace "ts-node
src/index.ts" with the actual entrypoint command used in this repo), or
remove/disable the nodemon config if it's unused — if you choose to keep
ts-node, ensure ts-node and a TypeScript entry file exist and are configured in
package.json/scripts so "ts-node src/index.ts" becomes valid.
---
Outside diff comments:
In `@acbu_burning/src/lib.rs`:
- Around line 1-21: Remove the duplicated soroban_sdk import block (the second
use ... block that re-declares contract, contractimpl, contracttype,
symbol_short, vec, Address, Env, IntoVal, Symbol, Vec/BytesN, SorobanString) so
there is only a single consolidated soroban_sdk import; also restore
CONTRACT_VERSION in the shared import list so the symbol used in initialize
(CONTRACT_VERSION) is available (add CONTRACT_VERSION back to the use
shared::{...} import alongside the other shared symbols).
- Around line 323-383: In redeem_basket: duplicate blocks caused double lookups
and double transfers—remove the repeated retrievals and duplicated logic so each
currency loop iteration only fetches currency, recipient, weight, rate, and
stoken once (use the first set of calls to CurrencyCode list, recipients.get(i),
env.invoke_contract calls for ORACLE_GET_BASKET_WEIGHT, ORACLE_GET_RATE,
ORACLE_GET_S_TOKEN_ADDR) and eliminate the second duplicated block including the
extra token.transfer_from; also move the recipient bounds check (i >=
recipients.len()) before any recipients.get(i) and keep only one
token.transfer_from call for native_i (or one transfer_from pair if two
different spender/vault semantics are required but not duplicated). Ensure
variables used later (acbu_gross_i, fee_i, native_i) come from the single
non-duplicated computation.
In `@acbu_escrow/src/lib.rs`:
- Line 199: The call in release() uses Self::get_admin(&env) but get_admin
expects Env by value, causing a type mismatch; replace that call with
Self::load_admin(&env) (which accepts &Env) to match other admin-gated methods
and fix the signature mismatch involving Env and functions get_admin and
load_admin.
- Around line 8-24: The EscrowError enum is missing variants referenced
elsewhere (EscrowError::NoPendingAdmin, EscrowError::AdminTimelockNotElapsed,
EscrowError::NoPendingAdminToCancel); add these three variants to the
EscrowError enum (e.g., assign unique u32 discriminants like 3012, 3013, 3014)
so the admin rotation methods compile, keep the #[repr(u32)] and derive
attributes intact, and ensure the new names exactly match the symbols used in
the codebase.
In `@acbu_lending_pool/src/lib.rs`:
- Around line 99-116: The Error enum is missing variants referenced by the
admin-rotation logic (Error::NoPendingAdmin, Error::AdminTimelockNotElapsed,
Error::NoPendingAdminToCancel), causing compile errors; open the enum definition
(pub enum Error) and add these three variants with unique repr(u32)
discriminants (e.g. next unused values such as 2005, 2006, 2007 or similar) so
they do not collide with existing codes, preserving #[contracterror] and the
derive attributes and keeping the enum ordering/format consistent.
In `@acbu_minting/src/lib.rs`:
- Around line 36-54: DataKey is missing the pending_admin and
pending_admin_eligible_at symbols referenced elsewhere; add two new pub fields
to the DataKey struct named pending_admin and pending_admin_eligible_at (types
matching other keys, e.g., Symbol) and then update the DATA_KEY const to include
corresponding entries for DATA_KEY.pending_admin and
DATA_KEY.pending_admin_eligible_at so the admin rotation methods can access
them.
- Around line 84-107: The MintingError enum is missing variants referenced
elsewhere (MaxSupplyExceeded, NoPendingAdmin, AdminTimelockNotElapsed,
NoPendingAdminToCancel) which breaks compilation; add these variants to the
MintingError enum (keeping #[repr(u32)] and non-duplicating discriminant
values—e.g., next unused IDs after InvalidVersion like 5019–5022 or similar) so
references like MintingError::MaxSupplyExceeded, MintingError::NoPendingAdmin,
MintingError::AdminTimelockNotElapsed, and MintingError::NoPendingAdminToCancel
resolve; ensure the new names exactly match usages and update any
documentation/comments if present.
- Around line 1-3: Move the crate attribute to the very top of the file (place
#![no_std] as the first line) and remove or fix the orphan,
syntactically-invalid stub before it: delete the stray "use
soroban_sdk::{Address, Env, String}; fn generate_unique_tx_id(...)" lines or
relocate them after the attribute and implement generate_unique_tx_id properly
(keep the function name generate_unique_tx_id and ensure it returns a
soroban_sdk::String using String::from_str(env, prefix) with a correct signature
and parameter usage).
In `@acbu_reserve_tracker/src/lib.rs`:
- Around line 16-23: The ReserveTrackerError enum is missing variants referenced
elsewhere (ZeroSupply, NoPendingAdmin, AdminTimelockNotElapsed,
NoPendingAdminToCancel); add these four variants to the ReserveTrackerError enum
so references like ReserveTrackerError::ZeroSupply, ::NoPendingAdmin,
::AdminTimelockNotElapsed and ::NoPendingAdminToCancel compile, and assign each
a unique u32 discriminant consistent with the existing values (e.g., continuing
the 8001/8002/8999 pattern) to avoid collisions; update the #[repr(u32)] enum
ReserveTrackerError declaration accordingly.
In `@acbu_savings_vault/src/lib.rs`:
- Around line 1-8: There are duplicate import blocks from soroban_sdk (the two
separate use lines importing contract, contracterror, contractimpl,
contracttype, symbol_short, Address, BytesN, Env, Symbol, Vec) which will cause
compilation errors; fix by consolidating them into a single use
soroban_sdk::{...} statement that includes each required symbol (e.g., contract,
contracterror, contractimpl, contracttype, symbol_short, Address, BytesN, Env,
Symbol, Vec) and remove the redundant import block so only one unified import
exists in lib.rs.
- Around line 61-70: DATA_KEY is missing initializers for the DataKey fields
pending_admin and pending_admin_eligible_at; update the DATA_KEY const to
include values for pending_admin and pending_admin_eligible_at (use
symbol_short! or the same convention as other keys, e.g.,
symbol_short!("pending_admin") and symbol_short!("pending_admin_time") or
similar meaningful short symbols) so the DataKey struct initialization in
DATA_KEY matches the struct definition (refer to DataKey, DATA_KEY,
pending_admin, pending_admin_eligible_at, and symbol_short!).
- Around line 22-42: The Error enum is missing a variant referenced elsewhere
(Error::NoPendingAdminToCancel) and cancel_upgrade was incorrectly handling
admin cancellation; add distinct error variants and functions: add
Error::NoPendingAdminToCancel (or rename to Error::NoPendingUpgradeToCancel if
intent is upgrade cancellation) to the Error enum and split logic so
cancel_upgrade only checks and returns the upgrade-related error (e.g.,
Error::NoPendingUpgradeToCancel), while admin rotation code lives in a separate
cancel_admin_transfer function that returns Error::NoPendingAdminToCancel when
appropriate; update callers of cancel_upgrade and any match arms to use the
correct variant names (Error, cancel_upgrade, cancel_admin_transfer).
- Around line 72-91: Remove the duplicate local definitions that conflict with
imports: delete the local SharedDataKey enum and the duplicate Error enum in
this file and rely on the already-imported shared::SharedDataKey and the
existing Error definition (the original one at lines 22-42) instead; update any
references in this module to use the imported SharedDataKey and the single Error
type (and keep DepositLot if it is unique) so there are no shadowing or
duplicate-type compilation errors.
- Around line 415-446: The file has a duplicated load_admin definition and a
mangled cancel_upgrade placed inside the first load_admin; fix by removing the
duplicate/partial load_admin and restoring a single fn load_admin(&Env) ->
Result<Address, Error> that only reads DATA_KEY.admin and returns Ok or
Error::NoAdmin, then implement pub fn cancel_upgrade(env: Env) as its own
function that: calls Self::load_admin(&env).unwrap_or_else(|e|
env.panic_with_error(e)), requires admin.require_auth(), removes
DATA_KEY.pending_upgrade and DATA_KEY.pending_upgrade_eligible_at (not
pending_admin keys), and publishes the cancellation event (use the existing
event pattern and include admin, pending_upgrade address/value, and timestamp).
Ensure you reference and use the symbols load_admin, cancel_upgrade,
DATA_KEY.pending_upgrade, and DATA_KEY.pending_upgrade_eligible_at.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a42bece7-c1a8-4bca-be26-26673e262d9e
📒 Files selected for processing (7)
acbu_burning/src/lib.rsacbu_escrow/src/lib.rsacbu_lending_pool/src/lib.rsacbu_minting/src/lib.rsacbu_reserve_tracker/src/lib.rsacbu_savings_vault/src/lib.rsnodemon.json
Comment on lines
359
to
381
| pub fn pause(env: Env) { | ||
| let admin = Self::load_admin(&env).unwrap_or_else(|e| env.panic_with_error(e)); | ||
| admin.require_auth(); | ||
| env.storage().instance().set(&DATA_KEY.paused, &true); | ||
| let current_version: u32 = env.storage().instance().get(&SharedDataKey::Version).unwrap_or(0); | ||
| if new_version <= current_version { env.panic_with_error(Error::InvalidVersion); } | ||
|
|
||
| env.storage().instance().set(&DATA_KEY.pending_upgrade_wasm, &new_wasm_hash); | ||
| env.storage().instance().set(&DATA_KEY.pending_upgrade_version, &new_version); | ||
| env.storage().instance().set(&DATA_KEY.pending_upgrade_eligible_at, &(env.ledger().timestamp() + UPGRADE_TIMELOCK_SECONDS)); | ||
| } | ||
|
|
||
| pub fn unpause(env: Env) { | ||
| let admin = Self::load_admin(&env).unwrap_or_else(|e| env.panic_with_error(e)); | ||
| pub fn execute_upgrade(env: Env) { | ||
| let admin = Self::load_admin(&env).unwrap_or_else(|_| env.panic_with_error(Error::NoAdmin)); | ||
| admin.require_auth(); | ||
| env.storage().instance().set(&DATA_KEY.paused, &false); | ||
| } | ||
|
|
||
| pub fn get_version(env: Env) -> u32 { | ||
| env.storage() | ||
| .instance() | ||
| .get(&SharedDataKey::Version) | ||
| .unwrap_or(0) | ||
| let wasm_hash: BytesN<32> = env.storage().instance().get(&DATA_KEY.pending_upgrade_wasm).unwrap_or_else(|| env.panic_with_error(Error::NoPendingUpgrade)); | ||
| let new_version: u32 = env.storage().instance().get(&DATA_KEY.pending_upgrade_version).unwrap_or(0); | ||
| env.deployer().update_current_contract_wasm(wasm_hash); | ||
| env.storage().instance().set(&SharedDataKey::Version, &new_version); | ||
| env.storage().instance().remove(&DATA_KEY.pending_upgrade_wasm); | ||
| env.storage().instance().remove(&DATA_KEY.pending_upgrade_version); | ||
| env.storage().instance().remove(&DATA_KEY.pending_upgrade_eligible_at); | ||
| } |
There was a problem hiding this comment.
pause() function body is corrupted with upgrade logic; execute_upgrade defined twice.
The pause() function (lines 359-368) contains upgrade proposal logic instead of pausing logic. It should simply set the paused flag to true.
Additionally, execute_upgrade is defined twice (lines 370-381 and 402-413).
🐛 Proposed fix for pause()
pub fn pause(env: Env) {
let admin = Self::load_admin(&env).unwrap_or_else(|e| env.panic_with_error(e));
admin.require_auth();
- let current_version: u32 = env.storage().instance().get(&SharedDataKey::Version).unwrap_or(0);
- if new_version <= current_version { env.panic_with_error(Error::InvalidVersion); }
-
- env.storage().instance().set(&DATA_KEY.pending_upgrade_wasm, &new_wasm_hash);
- env.storage().instance().set(&DATA_KEY.pending_upgrade_version, &new_version);
- env.storage().instance().set(&DATA_KEY.pending_upgrade_eligible_at, &(env.ledger().timestamp() + UPGRADE_TIMELOCK_SECONDS));
+ env.storage().instance().set(&DATA_KEY.paused, &true);
}And remove the first duplicate execute_upgrade (lines 370-381).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@acbu_savings_vault/src/lib.rs` around lines 359 - 381, The pause() function
body currently contains upgrade proposal logic; replace that body so pause()
simply sets the contract paused flag (e.g., set &DATA_KEY.paused or
&SharedDataKey::Paused to true) and performs admin.require_auth() as needed,
removing any references to new_version/new_wasm_hash/UPGRADE_TIMELOCK_SECONDS;
also delete the first duplicate execute_upgrade function (the one that updates
pending_upgrade_* storage and calls update_current_contract_wasm) and keep the
correct execute_upgrade implementation only once.
Comment on lines
+1
to
+6
| { | ||
| "watch": ["src"], | ||
| "ext": "ts,js,json", | ||
| "ignore": ["dist", "node_modules"], | ||
| "exec": "ts-node src/index.ts" | ||
| } |
There was a problem hiding this comment.
PR title contradicts the actual configuration.
The PR title states "Nodemon watches dist/ directory," but the configuration watches src and explicitly ignores dist. Consider updating the PR title to accurately reflect that nodemon watches the src directory, not dist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodemon.json` around lines 1 - 6, The PR title is inaccurate: nodemon.json's
"watch" key is set to "src" and "ignore" includes "dist", so update the PR title
to reflect that nodemon watches the src directory (not dist); search for the
"watch" and "ignore" entries in nodemon.json to confirm and then change the PR
title to something like "Nodemon watches src/ directory" or an equivalent
accurate description.
| "watch": ["src"], | ||
| "ext": "ts,js,json", | ||
| "ignore": ["dist", "node_modules"], | ||
| "exec": "ts-node src/index.ts" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if src/index.ts exists and find potential TypeScript entry points
echo "=== Checking for src/index.ts ==="
if [ -f "src/index.ts" ]; then
echo "✓ src/index.ts exists"
echo "First 10 lines:"
head -n 10 src/index.ts
else
echo "✗ src/index.ts NOT FOUND"
fi
echo -e "\n=== Looking for potential TypeScript entry points in src/ ==="
fd -e ts -e js . src/ --max-depth 2 --type f | head -20
echo -e "\n=== Checking package.json for main/start scripts ==="
if [ -f "package.json" ]; then
jq -r '.main // "not set", .scripts.start // "not set", .scripts.dev // "not set"' package.json 2>/dev/null || cat package.json | grep -E '"(main|start|dev)"'
fiRepository: Pi-Defi-world/acbu-smart-contract
Length of output: 363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== nodemon.json ==="
ls -la
cat nodemon.json
echo -e "\n=== package.json (relevant fields) ==="
cat package.json | jq -r '{main, scripts: .scripts}' 2>/dev/null || true
rg -n '"(main|start|dev|watch|nodemon)"' package.json || true
echo -e "\n=== Locate TypeScript entry candidates ==="
# Find any index.ts files anywhere
fd -e ts -e js -e mjs -e cjs -t f 'index.ts' . | head -50
# Find top-level src directories containing ts files (search for any src/* .ts)
fd -e ts . . | head -50
echo -e "\n=== Check for ts-node targets referenced in repo ==="
rg -n 'ts-node\s+[^"\x27]+' . || true
rg -n 'ts-node\s+.*(index\.ts|src\/index\.ts|main\.ts)' . || trueRepository: Pi-Defi-world/acbu-smart-contract
Length of output: 3359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for any package.json files ==="
fd -t f 'package.json' . | head -100
echo -e "\n=== Search for any nodemon.json files ==="
fd -t f 'nodemon.json' . | head -100
echo -e "\n=== Check whether a root-level src/ directory exists ==="
if [ -d "src" ]; then ls -la src; else echo "✗ No root-level src/ directory"; fi
echo -e "\n=== Look for src/index.ts anywhere in the repo ==="
# direct match
fd -t f -e ts 'index.ts' . | rg -n '(^|/+)src/index\.ts$' || true
# broad hints
fd -t f -e ts 'index.ts' . | head -50
echo -e "\n=== Look for references to ts-node entrypoints ==="
rg -n --hidden --no-ignore -S 'ts-node\s+.*(index\.ts|src/index\.ts)' . || true
rg -n --hidden --no-ignore -S '"exec"\s*:\s*"ts-node' . || true
echo -e "\n=== Look for any top-level TS files (ts, js) ==="
fd -t f -e ts . | head -50
fd -t f -e js . | head -50Repository: Pi-Defi-world/acbu-smart-contract
Length of output: 546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Count/list all .ts files ==="
count_ts=$(fd -t f -e ts . | wc -l)
echo "Total .ts files: $count_ts"
fd -t f -e ts . | head -200
echo -e "\n=== Look specifically for index.ts ==="
count_index=$(fd -t f -e ts 'index.ts' . | wc -l)
echo "Total index.ts files: $count_index"
fd -t f -e ts 'index.ts' . | head -50
echo -e "\n=== Look for ts-node usage ==="
rg -n --hidden --no-ignore -S 'ts-node' . || true
echo -e "\n=== Look for package.json anywhere ==="
pkgs=$(fd -t f 'package.json' . | wc -l)
echo "Total package.json files: $pkgs"
fd -t f 'package.json' . | head -50
echo -e "\n=== Look for nodemon usage in scripts/README ==="
rg -n --hidden --no-ignore -S 'nodemon' README* docs scripts . || trueRepository: Pi-Defi-world/acbu-smart-contract
Length of output: 393
Fix nodemon entrypoint (ts-node src/index.ts)—it points to a missing file
nodemon.json (line 5) runs ts-node src/index.ts, but the repo has no src/ directory, no .ts files at all, and no package.json; nodemon will fail on startup. Update exec/watch to the real entrypoint (and ensure ts-node is configured), or remove/disable this nodemon config if it isn’t used.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodemon.json` at line 5, The nodemon config's exec value "ts-node
src/index.ts" points to a non-existent entrypoint and will fail startup; update
nodemon.json to either set exec to the real runtime command for this project
(replace "ts-node src/index.ts" with the actual entrypoint command used in this
repo), or remove/disable the nodemon config if it's unused — if you choose to
keep ts-node, ensure ts-node and a TypeScript entry file exist and are
configured in package.json/scripts so "ts-node src/index.ts" becomes valid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.