Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions contracts/split-escrow/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ pub enum Error {
SplitNotPending = 7,
SplitNotReady = 8,
TreasuryNotSet = 9,
InvalidVersion = 10,
}
7 changes: 7 additions & 0 deletions contracts/split-escrow/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,10 @@ pub fn emit_fees_collected(env: &Env, amount: i128, treasury: &Address) {
(amount, treasury.clone()),
);
}

pub fn emit_contract_upgraded(env: &Env, old_version: soroban_sdk::String, new_version: soroban_sdk::String) {
env.events().publish(
(Symbol::new(env, "ContractUpgraded"),),
(old_version, new_version),
);
}
4 changes: 2 additions & 2 deletions contracts/split-escrow/src/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ pub fn calculate_fee(total: i128, fee_bps: u32) -> i128 {

pub fn collect_fee(env: &Env, total: i128) -> Result<i128, Error> {
let fee_bps = storage::get_fee_bps(env);
let treasury = storage::get_treasury(env).ok_or(Error::TreasuryNotSet)?;
let fee_amount = calculate_fee(total, fee_bps);

if fee_amount > 0 {
let treasury = storage::get_treasury(env).ok_or(Error::TreasuryNotSet)?;
let token = storage::get_token(env);
let token_client = token::Client::new(env, &token);
token_client.transfer(&env.current_contract_address(), &treasury, &fee_amount);
events::emit_fees_collected(env, fee_amount, &treasury);
}

events::emit_fees_collected(env, fee_amount, &treasury);
Ok(fee_amount)
}
62 changes: 61 additions & 1 deletion contracts/split-escrow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,23 @@ pub struct SplitEscrowContract;

#[contractimpl]
impl SplitEscrowContract {
pub fn initialize(env: Env, admin: Address, token_address: Address) -> Result<(), Error> {
pub fn initialize(
env: Env,
admin: Address,
token_address: Address,
version: String,
) -> Result<(), Error> {
if storage::has_admin(&env) {
return Err(Error::AlreadyInitialized);
}
if !is_valid_semver(&version) {
return Err(Error::InvalidVersion);
}
admin.require_auth();
storage::set_admin(&env, &admin);
storage::set_token(&env, &token_address);
storage::set_fee_bps(&env, 0u32);
storage::set_version(&env, &version);
events::emit_initialized(&env, &admin);
Ok(())
}
Expand Down Expand Up @@ -126,4 +135,55 @@ impl SplitEscrowContract {
pub fn get_split(env: Env, split_id: u64) -> Result<Split, Error> {
storage::get_split(&env, split_id).ok_or(Error::SplitNotFound)
}

pub fn get_version(env: Env) -> String {
storage::get_version(&env)
}

pub fn upgrade_version(env: Env, new_version: String) -> Result<(), Error> {
if !storage::has_admin(&env) {
return Err(Error::NotInitialized);
}
let admin = storage::get_admin(&env);
admin.require_auth();

if !is_valid_semver(&new_version) {
return Err(Error::InvalidVersion);
}

let old_version = storage::get_version(&env);
storage::set_version(&env, &new_version);
events::emit_contract_upgraded(&env, old_version, new_version);
Ok(())
}
}

fn is_valid_semver(version: &String) -> bool {
let mut dot_count = 0;
let mut segment_has_digit = false;
let len = version.len() as usize;

if len == 0 || len > 16 {
return false;
}

let mut buf = [0u8; 16];
version.copy_into_slice(&mut buf[..len]);

for i in 0..len {
let char = buf[i];
if char == b'.' {
if !segment_has_digit {
return false;
}
dot_count += 1;
segment_has_digit = false;
} else if char >= b'0' && char <= b'9' {
segment_has_digit = true;
} else {
return false;
}
}

dot_count == 2 && segment_has_digit
}
9 changes: 9 additions & 0 deletions contracts/split-escrow/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub enum DataKey {
Split(u64),
FeeBps,
Treasury,
Version,
}

pub fn has_admin(env: &Env) -> bool {
Expand Down Expand Up @@ -75,3 +76,11 @@ pub fn set_treasury(env: &Env, treasury: &Address) {
pub fn get_treasury(env: &Env) -> Option<Address> {
env.storage().instance().get(&DataKey::Treasury)
}

pub fn set_version(env: &Env, version: &soroban_sdk::String) {
env.storage().instance().set(&DataKey::Version, version);
}

pub fn get_version(env: &Env) -> soroban_sdk::String {
env.storage().instance().get(&DataKey::Version).unwrap()
}
78 changes: 77 additions & 1 deletion contracts/split-escrow/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn setup() -> (

let contract_id = env.register_contract(None, SplitEscrowContract);
let client = SplitEscrowContractClient::new(&env, &contract_id);
client.initialize(&admin, &token);
client.initialize(&admin, &token, &String::from_str(&env, "1.0.0"));

token_admin_client.mint(&participant, &1_000_000);

Expand Down Expand Up @@ -117,3 +117,79 @@ fn test_fees_collected_event_emitted() {
let after_len = env.events().all().len();
assert!(after_len > before_len);
}

#[test]
fn test_get_version() {
let (env, client, _, _, _, _, _) = setup();
assert_eq!(client.get_version(), String::from_str(&env, "1.0.0"));
}

#[test]
fn test_upgrade_version_success() {
let (env, client, admin, _, _, _, _) = setup();

// Auth is mocked in setup, but we can explicitly test it if needed.
// Here we just test the functionality.
let new_version = String::from_str(&env, "1.1.0");
client.upgrade_version(&new_version);

assert_eq!(client.get_version(), new_version);
}

#[test]
#[should_panic(expected = "HostError: Error(Contract, #10)")]
fn test_upgrade_version_invalid_format() {
let (env, client, _, _, _, _, _) = setup();
let invalid_version = String::from_str(&env, "one.zero.zero");
client.upgrade_version(&invalid_version);
}

#[test]
fn test_release_funds_works_without_treasury_when_fee_is_zero() {
let (env, client, _admin, creator, participant, token_client, _) = setup();

// Fee is 0% by default after init. Treasury is NOT set.
let split_id = client.create_split(&creator, &String::from_str(&env, "No Fee"), &1_000);
client.deposit(&split_id, &participant, &1_000);

// This should NOT panic now, even without treasury set.
client.release_funds(&split_id);

assert_eq!(token_client.balance(&creator), 1_000);
let split = client.get_split(&split_id);
assert_eq!(split.status, SplitStatus::Released);
}

#[test]
#[should_panic]
fn test_upgrade_version_non_admin() {
let env = Env::default();
// We don't call mock_all_auths() here.

let admin = Address::generate(&env);
let token = Address::generate(&env);
let contract_id = env.register_contract(None, SplitEscrowContract);
let client = SplitEscrowContractClient::new(&env, &contract_id);
client.initialize(&admin, &token, &String::from_str(&env, "1.0.0"));

let non_admin = Address::generate(&env);
// This should panic because non_admin is not the stored admin and we didn't mock auth.
// However, since Soroban verification requires the caller to be authorized,
// calling this as non_admin without mocking admin's auth will fail.
env.as_contract(&non_admin, || {
client.upgrade_version(&String::from_str(&env, "2.0.0"));
});
}

#[test]
fn test_semver_validation_at_init() {
let env = Env::default();
let admin = Address::generate(&env);
let token = Address::generate(&env);
let contract_id = env.register_contract(None, SplitEscrowContract);
let client = SplitEscrowContractClient::new(&env, &contract_id);

// Valid
client.initialize(&admin, &token, &String::from_str(&env, "0.1.0"));
assert_eq!(client.get_version(), String::from_str(&env, "0.1.0"));
Comment on lines +184 to +194
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing env.mock_all_auths() before initialize.

The initialize function calls admin.require_auth(). Without mocking authentication, this test should fail on the auth check before reaching the semver validation. Add env.mock_all_auths() to ensure the test validates what it claims.

🐛 Proposed fix
 #[test]
 fn test_semver_validation_at_init() {
     let env = Env::default();
+    env.mock_all_auths();
     let admin = Address::generate(&env);
     let token = Address::generate(&env);
     let contract_id = env.register_contract(None, SplitEscrowContract);
     let client = SplitEscrowContractClient::new(&env, &contract_id);
     
     // Valid
     client.initialize(&admin, &token, &String::from_str(&env, "0.1.0"));
     assert_eq!(client.get_version(), String::from_str(&env, "0.1.0"));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[test]
fn test_semver_validation_at_init() {
let env = Env::default();
let admin = Address::generate(&env);
let token = Address::generate(&env);
let contract_id = env.register_contract(None, SplitEscrowContract);
let client = SplitEscrowContractClient::new(&env, &contract_id);
// Valid
client.initialize(&admin, &token, &String::from_str(&env, "0.1.0"));
assert_eq!(client.get_version(), String::from_str(&env, "0.1.0"));
#[test]
fn test_semver_validation_at_init() {
let env = Env::default();
env.mock_all_auths();
let admin = Address::generate(&env);
let token = Address::generate(&env);
let contract_id = env.register_contract(None, SplitEscrowContract);
let client = SplitEscrowContractClient::new(&env, &contract_id);
// Valid
client.initialize(&admin, &token, &String::from_str(&env, "0.1.0"));
assert_eq!(client.get_version(), String::from_str(&env, "0.1.0"));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contracts/split-escrow/src/test.rs` around lines 158 - 168, The test
test_semver_validation_at_init is missing a mocked auth context so initialize
(which calls admin.require_auth()) will fail before semver checks; fix by
calling env.mock_all_auths() at the start of the test before invoking
SplitEscrowContractClient::initialize (or before any admin.require_auth() path)
so the auth check is satisfied and the semver validation/assertion can run.

}
Loading