Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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),
);
}
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()
}
52 changes: 51 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,53 @@ 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]
#[should_panic] // Should fail due to require_auth failure for non-admin
fn test_upgrade_version_non_admin() {
let (env, client, _, _, _, _, _) = setup();
let non_admin = Address::generate(&env);

env.as_contract(&client.address, || {
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.

}