Skip to content

fix: audit and replace unwrap() calls with error handling #542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
summer-0ma wants to merge 6 commits into
base: main
Choose a base branch
from
+128 −28
Open

fix: audit and replace unwrap() calls with error handling #542

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93ede63
fix: audit and replace unwrap() calls with error handling
summer-0ma Aug 19, 2026
d0f5289
Merge branch 'main' into audit/issue-534-unwrap-calls
summer-0ma Aug 19, 2026
e88386e
Merge branch 'main' into audit/issue-534-unwrap-calls
summer-0ma Aug 21, 2026
e9fdf2b
Merge branch 'main' into audit/issue-534-unwrap-calls
summer-0ma Aug 23, 2026
04688e2
Merge branch 'main' into audit/issue-534-unwrap-calls
summer-0ma Aug 25, 2026
caaccf7
fix(contracts): replace remaining unwrap() calls and fix formatting
collinsezedike Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 118 additions & 15 deletions packages/contracts/blend-adapter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ pub enum ContractError {
AlreadyInitialized = 1,
/// An intermediate arithmetic operation would overflow `i128`.
Overflow = 2,
/// A state-mutating call was made before `initialize`.
NotInitialized = 3,
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -167,11 +169,29 @@ impl MeridianBlendAdapter {
/// than assumed 1:1, so the vault's adapter-share accounting (`ADPT_SH`)
/// tracks genuine, appreciating shares instead of raw principal (#486).

@collinsezedike collinsezedike Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unwrap_or_else(|| panic_with_error!(...)) block is still duplicated ~20 times across all three files, worth collapsing into one helper now rather than after another round of edits touches all 20 sites again.

pub fn deposit(env: Env, amount: i128) -> i128 {
let vault: Address = env.storage().instance().get(&VAULT_KEY).unwrap();
let vault: Address = env
.storage()
.instance()

@collinsezedike collinsezedike Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test exercises deposit/withdraw/get_pool/accrue on a freshly-registered, uninitialized contract, so the new NotInitialized path this PR adds is never actually verified to fire.

.get(&VAULT_KEY)
.unwrap_or_else(|| {

@collinsezedike collinsezedike Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 6-line unwrap_or_else(|| { panic_with_error!(...); unreachable!() }) block is copy-pasted around 20 times across all three files. Since fixing the unreachable_code build error above means touching every one of those sites anyway, worth collapsing this into a single helper now, e.g. a small extension trait method like .get_or_not_initialized(&env), so future changes to this pattern are a one-location fix.

panic_with_error!(&env, ContractError::NotInitialized);
});
vault.require_auth();

let pool: Address = env.storage().instance().get(&POOL_KEY).unwrap();
let usdc: Address = env.storage().instance().get(&USDC_KEY).unwrap();
let pool: Address = env
.storage()
.instance()
.get(&POOL_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let usdc: Address = env
.storage()
.instance()
.get(&USDC_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});

let adapter = env.current_contract_address();

Expand All @@ -194,6 +214,7 @@ impl MeridianBlendAdapter {

let client = BlendPoolClient::new(&env, &pool);
let index = client.get_reserve(&usdc).config.index;
// Map.get() safely returns Option, defaulting to 0 if the index doesn't exist.
let b_tokens_before = client
.get_positions(&adapter)
.collateral
Expand All @@ -214,13 +235,17 @@ impl MeridianBlendAdapter {
],
);

// Map.get() safely returns Option, defaulting to 0 if the index doesn't exist.
let b_tokens_after = client
.get_positions(&adapter)
.collateral
.get(index)
.unwrap_or(0);
let b_tokens_credited = b_tokens_after - b_tokens_before;

// Instance storage read defaults to 0 if TOTAL_KEY hasn't been set, which is safe since
// initialize() sets this key to 0. This unwrap_or pattern is the idiomatic way to handle
// optional storage values in Soroban.
let prev: i128 = env.storage().instance().get(&TOTAL_KEY).unwrap_or(0);
env.storage().instance().set(&TOTAL_KEY, &(prev + amount));

Expand All @@ -233,11 +258,29 @@ impl MeridianBlendAdapter {
/// submitting. Returns the USDC amount actually delivered to `recipient`,
/// measured directly rather than assumed to equal the request (#489).
pub fn withdraw(env: Env, shares: i128, recipient: Address) -> i128 {
let vault: Address = env.storage().instance().get(&VAULT_KEY).unwrap();
let vault: Address = env
.storage()
.instance()
.get(&VAULT_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
vault.require_auth();

let pool: Address = env.storage().instance().get(&POOL_KEY).unwrap();
let usdc: Address = env.storage().instance().get(&USDC_KEY).unwrap();
let pool: Address = env
.storage()
.instance()
.get(&POOL_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let usdc: Address = env
.storage()
.instance()
.get(&USDC_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});

let adapter = env.current_contract_address();
let client = BlendPoolClient::new(&env, &pool);
Expand Down Expand Up @@ -268,6 +311,9 @@ impl MeridianBlendAdapter {
let after = usdc_client.balance(&recipient);
let delivered = after - before;

// Instance storage read defaults to 0 if TOTAL_KEY hasn't been set, which is safe since
// initialize() sets this key to 0. This unwrap_or pattern is the idiomatic way to handle
// optional storage values in Soroban.
let prev: i128 = env.storage().instance().get(&TOTAL_KEY).unwrap_or(0);
let remaining = if prev > delivered {
prev - delivered
Expand All @@ -290,13 +336,22 @@ impl MeridianBlendAdapter {
/// (`get_positions`) rather than self-tracking it, so there is no risk of
/// drift between the stored total and Blend's actual accounting.
pub fn accrue(env: Env) -> Result<(), ContractError> {
let pool: Address = env.storage().instance().get(&POOL_KEY).unwrap();
let usdc: Address = env.storage().instance().get(&USDC_KEY).unwrap();
let pool: Address = env
.storage()
.instance()
.get(&POOL_KEY)
.ok_or(ContractError::NotInitialized)?;

@collinsezedike collinsezedike Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting these from .unwrap() to .ok_or(NotInitialized)? changes refresh()'s behavior, not just its error type. refresh() (below) discards accrue()'s result via #[allow(unused_must_use)], so calling it on an uninitialized adapter used to panic (trap the transaction) and now silently does nothing. Worth having refresh() propagate or explicitly handle the error instead of swallowing it, so this doesn't become a quiet no-op.

let usdc: Address = env
.storage()
.instance()
.get(&USDC_KEY)
.ok_or(ContractError::NotInitialized)?;
let adapter = env.current_contract_address();

let client = BlendPoolClient::new(&env, &pool);
let reserve = client.get_reserve(&usdc);
let positions = client.get_positions(&adapter);
// Map.get() safely returns Option, defaulting to 0 if the index doesn't exist.
let b_tokens = positions.collateral.get(reserve.config.index).unwrap_or(0);

let current_value = b_tokens_to_usdc(b_tokens, reserve.data.b_rate)?;
Expand All @@ -318,12 +373,20 @@ impl MeridianBlendAdapter {
/// yield only as of the last `accrue()` call; call `accrue()` first for a
/// value that includes interest accrued since then.
pub fn total_assets(env: Env) -> i128 {
// Instance storage read defaults to 0 if TOTAL_KEY hasn't been set, which is safe since
// initialize() sets this key to 0. This unwrap_or pattern is the idiomatic way to handle
// optional storage values in Soroban.
env.storage().instance().get(&TOTAL_KEY).unwrap_or(0)
}

/// Returns the Blend pool this adapter supplies to.
pub fn get_pool(env: Env) -> Address {
env.storage().instance().get(&POOL_KEY).unwrap()
env.storage()
.instance()
.get(&POOL_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
})
}

/// Returns "blend", identifying which protocol this adapter wraps.
Expand Down Expand Up @@ -391,8 +454,21 @@ mod tests {
to: Address,
requests: Vec<Request>,
) -> Val {
let scalar: i128 = env.storage().instance().get(&M_SCALAR).unwrap();
let rate: i128 = env.storage().instance().get(&M_RATE).unwrap();
// Scalar and rate are always set in initialize(), so these are safe.
let scalar: i128 = env
.storage()
.instance()
.get(&M_SCALAR)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let rate: i128 = env
.storage()
.instance()
.get(&M_RATE)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let mut collateral: i128 = env.storage().instance().get(&M_COLLAT).unwrap_or(0);

for req in requests.iter() {
Expand All @@ -417,14 +493,33 @@ mod tests {
}

pub fn get_reserve(env: Env, asset: Address) -> Reserve {
let internal_scalar: i128 = env.storage().instance().get(&M_SCALAR).unwrap();
// Scalar and rate are always set in initialize(), so these are safe.
let internal_scalar: i128 = env
.storage()
.instance()
.get(&M_SCALAR)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let scalar: i128 = env
.storage()
.instance()
.get(&M_REP_SCL)
.unwrap_or(internal_scalar);
let rate: i128 = env.storage().instance().get(&M_RATE).unwrap();
let index: u32 = env.storage().instance().get(&M_INDEX).unwrap();
let rate: i128 = env
.storage()
.instance()
.get(&M_RATE)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let index: u32 = env
.storage()
.instance()
.get(&M_INDEX)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
Reserve {
asset,
config: ReserveConfig {
Expand Down Expand Up @@ -456,7 +551,15 @@ mod tests {
}

pub fn get_positions(env: Env, _address: Address) -> Positions {
let index: u32 = env.storage().instance().get(&M_INDEX).unwrap();
// Index is always set in initialize(), so this is safe.
let index: u32 = env
.storage()
.instance()
.get(&M_INDEX)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
// Collateral safely defaults to 0 if not set yet, which is correct for a fresh adapter.
let collateral: i128 = env.storage().instance().get(&M_COLLAT).unwrap_or(0);
let mut collateral_map = Map::new(&env);
collateral_map.set(index, collateral);
Expand Down
85 changes: 74 additions & 11 deletions packages/contracts/defindex-adapter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![no_std]

use soroban_sdk::{
contract, contractclient, contracterror, contractimpl, symbol_short, token::TokenClient, vec,
Address, Env, Symbol, Val, Vec,
contract, contractclient, contracterror, contractimpl, panic_with_error, symbol_short,
token::TokenClient, vec, Address, Env, Symbol, Val, Vec,
};

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -51,6 +51,8 @@ pub trait DefindexVaultInterface {
pub enum ContractError {
/// `initialize` was called on an adapter that already has a vault set.
AlreadyInitialized = 1,
/// A state-mutating call was made before `initialize`.
NotInitialized = 2,
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -83,10 +85,22 @@ impl MeridianDefindexAdapter {
/// Deposits USDC into the DeFindex vault on behalf of the adapter and
/// returns the dfToken shares received.
pub fn deposit(env: Env, amount: i128) -> i128 {
let vault: Address = env.storage().instance().get(&VAULT_KEY).unwrap();
let vault: Address = env
.storage()
.instance()
.get(&VAULT_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
vault.require_auth();

let dfx: Address = env.storage().instance().get(&DFX_VAULT).unwrap();
let dfx: Address = env
.storage()
.instance()
.get(&DFX_VAULT)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let adapter = env.current_contract_address();

let client = DefindexVaultClient::new(&env, &dfx);
Expand All @@ -101,16 +115,36 @@ impl MeridianDefindexAdapter {
/// DeFindex sends USDC to this adapter; the adapter forwards it to
/// `recipient`. Returns the USDC amount received.
pub fn withdraw(env: Env, shares: i128, recipient: Address) -> i128 {
let vault: Address = env.storage().instance().get(&VAULT_KEY).unwrap();
let vault: Address = env
.storage()
.instance()
.get(&VAULT_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
vault.require_auth();

let dfx: Address = env.storage().instance().get(&DFX_VAULT).unwrap();
let usdc: Address = env.storage().instance().get(&USDC_KEY).unwrap();
let dfx: Address = env
.storage()
.instance()
.get(&DFX_VAULT)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let usdc: Address = env
.storage()
.instance()
.get(&USDC_KEY)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let adapter = env.current_contract_address();

let amounts =
DefindexVaultClient::new(&env, &dfx).withdraw(&shares, &vec![&env, 0_i128], &adapter);

// Vec.get() returns Option which safely defaults to 0 if the vector doesn't contain
// index 0 or is empty, so this unwrap_or is safe and intentional.
let usdc_out: i128 = amounts.get(0).unwrap_or(0);
if usdc_out > 0 {
TokenClient::new(&env, &usdc).transfer(&adapter, &recipient, &usdc_out);
Expand All @@ -122,7 +156,13 @@ impl MeridianDefindexAdapter {
/// Live USDC value of the adapter's dfToken position, computed by the
/// DeFindex vault's exchange rate. Updates automatically as yield accrues.
pub fn total_assets(env: Env) -> i128 {
let dfx: Address = env.storage().instance().get(&DFX_VAULT).unwrap();
let dfx: Address = env
.storage()
.instance()
.get(&DFX_VAULT)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
let adapter = env.current_contract_address();

let client = DefindexVaultClient::new(&env, &dfx);
Expand All @@ -132,6 +172,8 @@ impl MeridianDefindexAdapter {
}

let amounts = client.get_asset_amounts_per_shares(&shares);
// Vec.get() returns Option which safely defaults to 0 if the vector doesn't contain
// index 0 or is empty, so this unwrap_or is safe and intentional.
amounts.get(0).unwrap_or(0)
}

Expand All @@ -141,7 +183,12 @@ impl MeridianDefindexAdapter {

/// Returns the DeFindex vault this adapter deposits into.
pub fn get_pool(env: Env) -> Address {
env.storage().instance().get(&DFX_VAULT).unwrap()
env.storage()
.instance()
.get(&DFX_VAULT)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
})
}

/// Returns "defindex", identifying which protocol this adapter wraps.
Expand Down Expand Up @@ -199,7 +246,15 @@ mod tests {
from: Address,
_invest: bool,
) -> Val {
let usdc: Address = env.storage().instance().get(&MDV_USDC).unwrap();
// USDC address is always set in initialize(), so this is safe.
let usdc: Address = env
.storage()
.instance()
.get(&MDV_USDC)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
// Vec.get() safely returns Option which defaults to 0, so unwrap_or is safe.
let amount = amounts_desired.get(0).unwrap_or(0);
TokenClient::new(&env, &usdc).transfer(&from, &env.current_contract_address(), &amount);

Expand All @@ -225,7 +280,15 @@ mod tests {
.get(&MDV_WAMT)
.unwrap_or_else(|| vec![&env, withdraw_shares]);

let usdc: Address = env.storage().instance().get(&MDV_USDC).unwrap();
// USDC address is always set in initialize(), so this is safe.
let usdc: Address = env
.storage()
.instance()
.get(&MDV_USDC)
.unwrap_or_else(|| {
panic_with_error!(&env, ContractError::NotInitialized);
});
// Vec.get() safely returns Option which defaults to 0, so unwrap_or is safe.
let payout = amounts.get(0).unwrap_or(0);
if payout > 0 {
TokenClient::new(&env, &usdc).transfer(
Expand Down
Loading