Skip to content

fix: contract error semantics, token validation, and O(1) swap lookups #387

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

Merged
cypriannwokolo2-creator merged 3 commits into from
Aug 31, 2026
Merged

fix: contract error semantics, token validation, and O(1) swap lookups #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04adf07
fix(governance-token): use AlreadyInitialized error for double init
MatteoRossi Aug 31, 2026
fc709cf
fix(treasury): validate token address is deployed contract in rescue_…
MatteoRossi Aug 31, 2026
752210d
perf(escrow-swap): use Map keyed by swap ID for O(1) lookup
MatteoRossi Aug 31, 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
167 changes: 64 additions & 103 deletions packages/escrow-swap/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use soroban_sdk::{token::Client as TokenClient, Address, BytesN, Env, Vec};
use soroban_sdk::{token::Client as TokenClient, Address, BytesN, Env, Map, Vec};
use crate::types::*;
use common::pause;

Expand All @@ -12,7 +12,7 @@ pub fn init(env: &Env, admin: &Address) -> Result<(), EscrowError> {
}
env.storage().instance().set(&DataKey::Admin, admin);
env.storage().instance().set(&DataKey::NextSwapId, &0u64);
env.storage().persistent().set(&DataKey::SwapRequests, &Vec::<SwapRequest>::new(env));
env.storage().persistent().set(&DataKey::SwapRequests, &Map::<u64, SwapRequest>::new(env));
Ok(())
}

Expand Down Expand Up @@ -84,8 +84,8 @@ pub fn create_swap(
created_at: now,
};

let mut swaps: Vec<SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).unwrap_or_else(|| Vec::new(env));
swaps.push_back(swap);
let mut swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).unwrap_or_else(|| Map::new(env));
swaps.set(next_id, swap);
env.storage().persistent().set(&DataKey::SwapRequests, &swaps);
env.storage().instance().set(&DataKey::NextSwapId, &next_id.checked_add(1).ok_or(EscrowError::InvalidAmount)?);

Expand Down Expand Up @@ -133,43 +133,32 @@ pub fn accept_swap(env: &Env, id: u64, responder: &Address, secret: BytesN<32>)
let _guard = common::reentrancy::ReentrancyGuard::new(env).map_err(|_| EscrowError::NotInitialized)?;
responder.require_auth();

let mut swaps: Vec<SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut found = false;

for i in 0..swaps.len() {
let mut s = swaps.get(i).ok_or(EscrowError::VecAccessError)?;
if s.id == id {
if s.status != STATUS_PENDING {
return Err(EscrowError::SwapNotActive);
}
if s.responder != *responder {
return Err(EscrowError::Unauthorized);
}
let secret_bytes = soroban_sdk::Bytes::from_array(env, &secret.to_array());
let computed_hash: BytesN<32> = env.crypto().sha256(&secret_bytes).into();
if computed_hash != s.hash_lock {
return Err(EscrowError::HashLockMismatch);
}
let now = env.ledger().timestamp();
if now >= s.time_lock {
return Err(EscrowError::TimeLockExpired);
}
let mut swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut s = swaps.get(id).ok_or(EscrowError::SwapNotFound)?;
if s.status != STATUS_PENDING {
return Err(EscrowError::SwapNotActive);
}
if s.responder != *responder {
return Err(EscrowError::Unauthorized);
}
let secret_bytes = soroban_sdk::Bytes::from_array(env, &secret.to_array());
let computed_hash: BytesN<32> = env.crypto().sha256(&secret_bytes).into();
if computed_hash != s.hash_lock {
return Err(EscrowError::HashLockMismatch);
}
let now = env.ledger().timestamp();
if now >= s.time_lock {
return Err(EscrowError::TimeLockExpired);
}

s.status = STATUS_ACTIVE;
swaps.set(i, s.clone());
found = true;
s.status = STATUS_ACTIVE;
swaps.set(id, s.clone());

let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(responder, &env.current_contract_address(), &s.responder_amount);
let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(responder, &env.current_contract_address(), &s.responder_amount);

SwapAccepted { id, responder: responder.clone() }.publish(env);
break;
}
}
SwapAccepted { id, responder: responder.clone() }.publish(env);

if !found {
return Err(EscrowError::SwapNotFound);
}
env.storage().persistent().set(&DataKey::SwapRequests, &swaps);
Ok(())
}
Expand All @@ -179,36 +168,25 @@ pub fn complete_swap(env: &Env, id: u64, caller: &Address) -> Result<(), EscrowE
let _guard = common::reentrancy::ReentrancyGuard::new(env).map_err(|_| EscrowError::NotInitialized)?;
caller.require_auth();

let mut swaps: Vec<SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut found = false;

for i in 0..swaps.len() {
let mut s = swaps.get(i).ok_or(EscrowError::VecAccessError)?;
if s.id == id {
if s.status != STATUS_ACTIVE {
return Err(EscrowError::SwapNotActive);
}
if *caller != s.initiator && *caller != s.responder {
return Err(EscrowError::Unauthorized);
}
s.status = STATUS_COMPLETED;
swaps.set(i, s.clone());
found = true;
let mut swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut s = swaps.get(id).ok_or(EscrowError::SwapNotFound)?;
if s.status != STATUS_ACTIVE {
return Err(EscrowError::SwapNotActive);
}
if *caller != s.initiator && *caller != s.responder {
return Err(EscrowError::Unauthorized);
}
s.status = STATUS_COMPLETED;
swaps.set(id, s.clone());

let token_a_client = TokenClient::new(env, &s.token_a);
token_a_client.transfer(&env.current_contract_address(), &s.responder, &s.initiator_amount);
let token_a_client = TokenClient::new(env, &s.token_a);
token_a_client.transfer(&env.current_contract_address(), &s.responder, &s.initiator_amount);

let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(&env.current_contract_address(), &s.initiator, &s.responder_amount);
let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(&env.current_contract_address(), &s.initiator, &s.responder_amount);

SwapCompleted { id, initiator: s.initiator.clone(), responder: s.responder.clone() }.publish(env);
break;
}
}
SwapCompleted { id, initiator: s.initiator.clone(), responder: s.responder.clone() }.publish(env);

if !found {
return Err(EscrowError::SwapNotFound);
}
env.storage().persistent().set(&DataKey::SwapRequests, &swaps);
Ok(())
}
Expand All @@ -218,58 +196,41 @@ pub fn cancel_swap(env: &Env, id: u64, caller: &Address) -> Result<(), EscrowErr
let _guard = common::reentrancy::ReentrancyGuard::new(env).map_err(|_| EscrowError::NotInitialized)?;
caller.require_auth();

let mut swaps: Vec<SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut found = false;

for i in 0..swaps.len() {
let mut s = swaps.get(i).ok_or(EscrowError::VecAccessError)?;
if s.id == id {
if s.status == STATUS_COMPLETED || s.status == STATUS_CANCELLED {
return Err(EscrowError::SwapNotActive);
}
if *caller != s.initiator && *caller != s.responder {
return Err(EscrowError::Unauthorized);
}

let previous_status = s.status;
s.status = STATUS_CANCELLED;
swaps.set(i, s.clone());
found = true;

let token_a_client = TokenClient::new(env, &s.token_a);
token_a_client.transfer(&env.current_contract_address(), &s.initiator, &s.initiator_amount);
let mut swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
let mut s = swaps.get(id).ok_or(EscrowError::SwapNotFound)?;
if s.status == STATUS_COMPLETED || s.status == STATUS_CANCELLED {
return Err(EscrowError::SwapNotActive);
}
if *caller != s.initiator && *caller != s.responder {
return Err(EscrowError::Unauthorized);
}

let previous_status = s.status;
s.status = STATUS_CANCELLED;
swaps.set(id, s.clone());

if previous_status == STATUS_ACTIVE {
let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(&env.current_contract_address(), &s.responder, &s.responder_amount);
}
let token_a_client = TokenClient::new(env, &s.token_a);
token_a_client.transfer(&env.current_contract_address(), &s.initiator, &s.initiator_amount);

SwapCancelled { id }.publish(env);
break;
}
if previous_status == STATUS_ACTIVE {
let token_b_client = TokenClient::new(env, &s.token_b);
token_b_client.transfer(&env.current_contract_address(), &s.responder, &s.responder_amount);
}

if !found {
return Err(EscrowError::SwapNotFound);
}
SwapCancelled { id }.publish(env);

env.storage().persistent().set(&DataKey::SwapRequests, &swaps);
Ok(())
}

pub fn get_swap(env: &Env, id: u64) -> Result<SwapRequest, EscrowError> {
let swaps: Vec<SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
for i in 0..swaps.len() {
if let Some(s) = swaps.get(i) {
if s.id == id {
return Ok(s);
}
}
}
Err(EscrowError::SwapNotFound)
let swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).ok_or(EscrowError::NotInitialized)?;
swaps.get(id).ok_or(EscrowError::SwapNotFound)
}

pub fn get_swaps(env: &Env) -> Vec<SwapRequest> {
env.storage().persistent().get(&DataKey::SwapRequests).unwrap_or_else(|| Vec::new(env))
let swaps: Map<u64, SwapRequest> = env.storage().persistent().get(&DataKey::SwapRequests).unwrap_or_else(|| Map::new(env));
swaps.values()
}

pub fn pause(env: &Env, admin: &Address) -> Result<(), EscrowError> {
Expand Down
2 changes: 1 addition & 1 deletion packages/governance-token/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub fn initialize(
decimals: u32,
) -> Result<(), TokenError> {
if env.storage().instance().has(&ADMIN_KEY) {
return Err(TokenError::NotInitialized);
return Err(TokenError::AlreadyInitialized);
}
env.storage().instance().set(&ADMIN_KEY, admin);
env.storage().instance().set(&META_KEY, &TokenMetadata {
Expand Down
2 changes: 1 addition & 1 deletion packages/governance-token/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ mod tests {
&String::from_str(&env, "TK2"),
&7u32,
);
assert!(result.is_err());
assert_eq!(result, Err(Ok(crate::types::TokenError::AlreadyInitialized)));
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions packages/governance-token/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub enum TokenError {
ContractPaused = 10,
Underflow = 11,
Frozen = 12,
AlreadyInitialized = 13,
}

#[contractevent]
Expand Down
4 changes: 4 additions & 0 deletions packages/treasury/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ pub fn rescue_tokens(
return Err(TreasuryError::InvalidAmount);
}

if !env.has_contract(token) {
return Err(TreasuryError::InvalidAmount);
}

let treasury_token: Address = env
.storage()
.instance()
Expand Down