Skip to content

Commit 2adb9b0

Browse files
fix(batch-wallet): convert duplicate-within-batch panic into per-item Failure
Closes VertexChainLabs#129 Replace the separate pre-scan/panic loop with inline duplicate detection during processing. Duplicates now return WalletCreateResult::Failure(owner, DuplicateWallet) instead of panicking, allowing partial success for the rest of the batch. - Remove separate pre-scan loop that panicked on duplicates - Add inline duplicate check during processing with graceful handling - Emit wallet_duplicate event per duplicate - Update test from should_panic to partial-success verification - Add test_batch_create_wallets_partial_success_with_duplicates (5 requests, 2 duplicates → 3 succeed, 2 fail)
1 parent 7c17074 commit 2adb9b0

1 file changed

Lines changed: 53 additions & 17 deletions

File tree

  • contracts/batch-wallet/src

contracts/batch-wallet/src/lib.rs

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -224,26 +224,36 @@ impl BatchWalletContract {
224224
let mut successful: u32 = 0;
225225
let mut failed: u32 = 0;
226226

227-
// Check for duplicates within the batch
227+
// Track addresses seen in *this batch* to handle duplicates gracefully
228228
let mut seen_addresses = Vec::new(&env);
229+
230+
// Process each request — duplicates within the batch yield
231+
// WalletCreateResult::Failure(owner, DuplicateWallet) instead of
232+
// panicking, so the rest of the batch can still succeed.
229233
for i in 0..requests.len() {
230234
let request = requests.get(i).unwrap();
231235
let owner = request.owner.clone();
232236

233-
// Check if this address was already seen in this batch
237+
// Check for duplicate within the batch
238+
let mut is_duplicate = false;
234239
for seen in seen_addresses.iter() {
235240
if seen == owner {
236-
BatchWalletEvents::wallet_duplicate(&env, &owner);
237-
panic_with_error!(&env, BatchWalletError::DuplicateWallet);
241+
is_duplicate = true;
242+
break;
238243
}
239244
}
240-
seen_addresses.push_back(owner);
241-
}
242245

243-
// Process each request
244-
for i in 0..requests.len() {
245-
let request = requests.get(i).unwrap();
246-
let owner = request.owner.clone();
246+
if is_duplicate {
247+
BatchWalletEvents::wallet_duplicate(&env, &owner);
248+
results.push_back(WalletCreateResult::Failure(
249+
owner,
250+
BatchWalletError::DuplicateWallet as u32,
251+
));
252+
failed += 1;
253+
continue;
254+
}
255+
256+
seen_addresses.push_back(owner.clone());
247257

248258
if let Some(_existing_wallet) = get_wallet(&env, owner.clone()) {
249259
results.push_back(WalletCreateResult::Failure(
@@ -503,20 +513,46 @@ mod tests {
503513
}
504514

505515
#[test]
506-
// BatchWalletError::DuplicateWallet = 7 (`#[repr(u32)]`) — keep in sync if enum is reordered.
507-
#[should_panic(expected = "Error(Contract, #7)")]
508-
fn test_batch_create_wallets_duplicate_in_batch() {
516+
fn test_batch_create_wallets_partial_success_with_duplicates() {
517+
// Send N+1 requests where M are duplicates → result reports failed=M,
518+
// others succeed, event wallet_duplicate emitted per duplicate.
509519
let (env, admin, client) = setup_test_env();
510520

511-
let owner = Address::generate(&env);
521+
let owner1 = Address::generate(&env);
512522
let owner2 = Address::generate(&env);
523+
let owner3 = Address::generate(&env);
513524

525+
// 5 requests: owner1, owner2, owner1 (duplicate), owner3, owner2 (duplicate)
514526
let mut requests: Vec<WalletCreateRequest> = Vec::new(&env);
515-
requests.push_back(create_wallet_request(&env, owner.clone()));
527+
requests.push_back(create_wallet_request(&env, owner1.clone()));
516528
requests.push_back(create_wallet_request(&env, owner2.clone()));
517-
requests.push_back(create_wallet_request(&env, owner.clone()));
529+
requests.push_back(create_wallet_request(&env, owner1.clone())); // duplicate
530+
requests.push_back(create_wallet_request(&env, owner3.clone()));
531+
requests.push_back(create_wallet_request(&env, owner2.clone())); // duplicate
532+
533+
let result = client.batch_create_wallets(&admin, &requests);
518534

519-
client.batch_create_wallets(&admin, &requests);
535+
// 5 total, 3 unique → 3 succeed, 2 duplicates fail
536+
assert_eq!(result.total_requests, 5);
537+
assert_eq!(result.successful, 3);
538+
assert_eq!(result.failed, 2);
539+
assert_eq!(result.results.len(), 5);
540+
541+
// Verify wallets were created for unique owners only
542+
assert!(client.get_wallet(&owner1).is_some());
543+
assert!(client.get_wallet(&owner2).is_some());
544+
assert!(client.get_wallet(&owner3).is_some());
545+
546+
// Verify duplicate entries are Failure with DuplicateWallet code (7)
547+
let mut duplicate_count = 0;
548+
for r in result.results.iter() {
549+
if let WalletCreateResult::Failure(_, code) = r {
550+
if code == BatchWalletError::DuplicateWallet as u32 {
551+
duplicate_count += 1;
552+
}
553+
}
554+
}
555+
assert_eq!(duplicate_count, 2);
520556
}
521557

522558
#[test]

0 commit comments

Comments
 (0)