Skip to content

Conversation

@nzamulov
Copy link
Contributor

@nzamulov nzamulov commented Nov 5, 2025

Challenge 7 Solution

Submitted by: @nzamulov
Challenge: Challenge 7

Description

This PR contains my solution for Challenge 7.

Changes

  • Added solution file to challenge-7/submissions/nzamulov/solution-template.go

Testing

  • Solution passes all test cases
  • Code follows Go best practices

Thank you for reviewing my submission! 🚀

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Walkthrough

Introduces a new Go package challenge7 implementing a thread-safe BankAccount type with deposit, withdraw, and transfer operations. Includes custom error types for validation and uses mutex-based concurrency control with deadlock-avoidant locking for multi-account transfers.

Changes

Cohort / File(s) Summary
BankAccount Implementation
challenge-7/submissions/nzamulov/solution-template.go
New file introducing BankAccount struct with ID, Owner, Balance, MinBalance, and mutex. Adds MaxTransactionAmount constant and four custom error types (AccountError, InsufficientFundsError, NegativeAmountError, ExceedsLimitError). Implements NewBankAccount() factory function, Deposit(), Withdraw(), and Transfer() methods with comprehensive input validation and deadlock-avoidant locking for concurrent operations.

Sequence Diagrams

sequenceDiagram participant User participant Transfer as Transfer(amt, target) participant Lock1 as Lock Account 1 participant Lock2 as Lock Account 2 participant Validation as Validation User->>Transfer: Call Transfer Transfer->>Validation: Check non-nil target, no self-transfer alt Validation fails Validation-->>Transfer: Error Transfer-->>User: Error returned else Validation passes Validation-->>Transfer: OK Transfer->>Lock1: Acquire lock (lower ID) Transfer->>Lock2: Acquire lock (higher ID) Lock1-->>Transfer: Acquired Lock2-->>Transfer: Acquired Transfer->>Validation: Check amount, balance, limit alt Balance check fails Validation-->>Transfer: InsufficientFundsError else All checks pass Transfer->>Transfer: Update balances Transfer-->>Lock2: Release Transfer-->>Lock1: Release Transfer-->>User: nil (success) end end 
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify the deadlock-avoidant locking strategy based on ID comparison is correctly implemented across all transfer paths
  • Ensure all error types are properly validated and returned in appropriate scenarios
  • Confirm mutex locks are properly acquired and released in all methods, including error paths
  • Validate that the MinBalance constraint is enforced in both Withdraw and Transfer operations

Possibly related PRs

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add solution for Challenge 7 by nzamulov' accurately reflects the main change: adding a Challenge 7 solution file by the specified contributor.
Description check ✅ Passed The description is directly related to the changeset, explaining that it contains the Challenge 7 solution, specifies the file location, and notes testing completion.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 231bd24 and e9a90af.

📒 Files selected for processing (1)
  • challenge-7/submissions/nzamulov/solution-template.go (1 hunks)
Comment on lines +138 to +146
first, second := a, target
if a.ID > target.ID {
first, second = target, a
}

first.mu.Lock()
defer first.mu.Unlock()
second.mu.Lock()
defer second.mu.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Deadlock risk when two accounts share the same ID

Lock ordering swaps only when a.ID > target.ID. If two distinct accounts happen to share the same ID, goroutine A transferring from account1→account2 locks in the sequence (account1, account2) while goroutine B transferring from account2→account1 also locks (account2, account1). Both goroutines then block forever waiting on the other mutex. Because ID uniqueness is not enforced anywhere, this deadlock is reachable at runtime. To make the ordering unambiguous, add a deterministic tie-breaker (e.g., pointer address) when IDs compare equal so every transfer uses the same lock order.

-import "sync" +import ( +"sync" +"unsafe" +) @@ -first, second := a, target -if a.ID > target.ID { -first, second = target, a -} +first, second := a, target +if a.ID > target.ID || +(a.ID == target.ID && uintptr(unsafe.Pointer(a)) > uintptr(unsafe.Pointer(target))) { +first, second = target, a +}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In challenge-7/submissions/nzamulov/solution-template.go around lines 138 to 146, the current lock-ordering swaps only when a.ID > target.ID which leaves a deadlock when two distinct accounts share the same ID; change the ordering logic to include a deterministic tie-breaker when IDs are equal (for example compare the account object pointer addresses or another unique stable value) so that for any pair of accounts you always choose the same first/second before locking; implement the comparison so if a.ID == target.ID you fall back to comparing uintptr(unsafe.Pointer(a)) vs uintptr(unsafe.Pointer(target)) (or another stable unique key) and then lock first then second in that determined order. 
@nzamulov nzamulov closed this Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant