Make create_for_owner! idempotent and race-safe

lib/wallets/models/wallet.rb

@@ -88,8 +88,12 @@ def self.transfer_class
     class << self
       def create_for_owner!(owner:, asset_code:, initial_balance: 0, metadata: {})
         initial_balance = normalize_initial_balance(initial_balance)
+        asset_code = normalize_asset_code(asset_code)
         metadata = metadata.respond_to?(:to_h) ? metadata.to_h : {}
 
+        existing_wallet = find_by(owner: owner, asset_code: asset_code)
+        return existing_wallet if existing_wallet.present?
+
         transaction do
           wallet = create!(
             owner: owner,
@@ -98,20 +102,32 @@ def create_for_owner!(owner:, asset_code:, initial_balance: 0, metadata: {})
             metadata: metadata
           )
 
-          if initial_balance.to_i.positive?
-            wallet.credit(
-              initial_balance,
-              category: :adjustment,
-              metadata: { reason: "initial_balance" }
-            )
+          if initial_balance.positive?
+            wallet.credit(initial_balance, **initial_balance_credit_attributes)
           end
 
           wallet
+        rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => error
+          wallet = find_by(owner: owner, asset_code: asset_code)
+          raise error if wallet.nil?
+
+          if record_conflict_due_to_existing_wallet?(error)
+            wallet
+          else
+            raise error
+          end
         end
       end
 
       private
 
+      def initial_balance_credit_attributes
+        {
+          category: :adjustment,
+          metadata: { reason: "initial_balance" }
+        }
+      end
+
       def normalize_initial_balance(value)
         return 0 if value.nil?
         raise ArgumentError, "Initial balance must be a whole number" unless value == value.to_i
@@ -121,6 +137,17 @@ def normalize_initial_balance(value)
 
         value
       end
+
+      def normalize_asset_code(value)
+        value.to_s.strip.downcase.presence || raise(ArgumentError, "Asset code is required")
+      end
+
+      def record_conflict_due_to_existing_wallet?(error)
+        return true if error.is_a?(ActiveRecord::RecordNotUnique)
+        return false unless error.is_a?(ActiveRecord::RecordInvalid)
+
+        error.record.errors.of_kind?(:asset_code, :taken)
+      end
     end
 
     # =========================================

test/models/wallets/wallet_test.rb

@@ -62,6 +62,31 @@ class Wallets::WalletTest < ActiveSupport::TestCase
     end
   end
 
+  test "create_for_owner is idempotent for an existing owner and asset" do
+    owner = users(:new_user)
+
+    wallet = Wallets::Wallet.create_for_owner!(
+      owner: owner,
+      asset_code: :ore,
+      initial_balance: 25
+    )
+
+    assert_no_difference -> { Wallets::Wallet.where(owner: owner, asset_code: "ore").count } do
+      same_wallet = Wallets::Wallet.create_for_owner!(
+        owner: owner,
+        asset_code: " ORE ",
+        initial_balance: 75
+      )
+
+      assert_equal wallet.id, same_wallet.id
+    end
+
+    assert_equal 25, wallet.reload.balance
+    assert_equal 1, wallet.transactions.count
+    assert_equal "adjustment", wallet.transactions.sole.category
+    assert_equal "initial_balance", wallet.transactions.sole.metadata["reason"]
+  end
+
   test "negative balances are tracked correctly when enabled" do
     original_setting = Wallets.configuration.allow_negative_balance
     Wallets.configuration.allow_negative_balance = true

wallets.gemspec

@@ -16,21 +16,40 @@ Gem::Specification.new do |spec|
 
   spec.metadata["allowed_push_host"] = "https://rubygems.org"
   spec.metadata["homepage_uri"] = spec.homepage
-  spec.metadata["source_code_uri"] = spec.homepage
+  spec.metadata["source_code_uri"] = "#{spec.homepage}/tree/main"
   spec.metadata["changelog_uri"] = "#{spec.homepage}/blob/main/CHANGELOG.md"
   spec.metadata["rubygems_mfa_required"] = "true"
 
-  spec.files = Dir.chdir(__dir__) do
-    Dir.glob("**/*", File::FNM_DOTMATCH).reject do |file|
-      file.start_with?(".git/", "coverage/", "dist/", "test/dummy/log/", "test/dummy/tmp/", "test/dummy/storage/") ||
-        file.end_with?(".gem") ||
-        [".", "..", ".DS_Store", "Gemfile.lock", "test/.DS_Store"].include?(file)
+  gemspec = File.basename(__FILE__)
+  spec.files = IO.popen(%w[git ls-files -z], chdir: __dir__, err: IO::NULL) do |ls|
+    ls.readlines("\x0", chomp: true).reject do |file|
+      (file == gemspec) ||
+        file.start_with?(*%w[
+          .aux/
+          .claude/
+          .cursor/
+          .github/
+          bin/
+          coverage/
+          dist/
+          gemfiles/
+          spec/
+          test/
+          tmp/
+        ]) ||
+        %w[
+          .DS_Store
+          .ruby-version
+          Appraisals
+          Gemfile
+          Gemfile.lock
+        ].include?(file)
     end
   end
 
   spec.bindir = "exe"
   spec.executables = spec.files.grep(%r{\Aexe/}) { |file| File.basename(file) }
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "rails", ">= 6.1"
+  spec.add_dependency "rails", ">= 6.1", "< 9.0"
 end