Factor Out Pending BTC Transaction Repo

Christopher Vollick created

The `bin/process_pending_btc_transactions` script was getting a bit
heavy, so I pulled out a piece of it into its own class with tests.

This, along with one or two lines that were split for length, now means
there's no rubocop violations in bin/process_pending_btc_transactions.

It's still a bit heavy and has a few internal classes that are untested,
but it's still progress!

Change summary

bin/process_pending_btc_transactions  |  46 +++++---
lib/electrum.rb                       |   2 
lib/pending_transaction_repo.rb       |  62 ++++++++++++
test/test_pending_transaction_repo.rb | 145 +++++++++++++++++++++++++++++
4 files changed, 236 insertions(+), 19 deletions(-)

Detailed changes

bin/process_pending_btc_transactions 🔗

@@ -25,6 +25,7 @@ require "redis"
 
 require_relative "../lib/blather_notify"
 require_relative "../lib/electrum"
+require_relative "../lib/pending_transaction_repo"
 require_relative "../lib/transaction"
 
 CONFIG =
@@ -61,8 +62,12 @@ when "btc"
 	exchange_row = canadianbitcoins.at("#ticker > table > tbody > tr")
 	raise "Bitcoin row has moved" unless exchange_row.at("td").text == "Bitcoin"
 when "bch"
-	exchange_row = canadianbitcoins.at("#ticker > table > tbody > tr:nth-child(2)")
-	raise "Bitcoin Cash row has moved" unless exchange_row.at("td").text == "Bitcoin Cash"
+	exchange_row = canadianbitcoins.at(
+		"#ticker > table > tbody > tr:nth-child(2)"
+	)
+	if exchange_row.at("td").text != "Bitcoin Cash"
+		raise "Bitcoin Cash row has moved"
+	end
 else
 	raise "Unknown currency #{CONFIG[:electrum][:currency]}"
 end
@@ -212,36 +217,39 @@ class Customer
 	end
 end
 
-done = REDIS.hgetall("pending_#{CONFIG[:electrum][:currency]}_transactions").map { |(txid, customer_id)|
-	tx_hash, address = txid.split("/", 2)
+repo = PendingTransactionRepo.new(
+	"pending_#{CONFIG[:electrum][:currency]}_transactions"
+)
 
-	transaction = begin
-		ELECTRUM.gettransaction(tx_hash)
-	rescue Electrum::NoTransaction
-		warn $!.to_s
-		next
+repo.error_handler do |e|
+	case e
+	when Electrum::NoTransaction
+		warn e.to_s
+		true # Skip and continue
 	end
+end
 
-	next unless transaction.confirmations >= CONFIG[:required_confirmations]
+repo.map do |pending, customer_id|
+	next unless pending.confirmations >= CONFIG[:required_confirmations]
 
-	btc = transaction.amount_for(address)
-	if btc <= 0
+	if pending.outgoing?
 		# This is a send, not a receive, do not record it
-		REDIS.hdel("pending_#{CONFIG[:electrum][:currency]}_transactions", txid)
+		repo.remove_transaction(pending)
 		next
 	end
 	DB.transaction do
 		customer = Customer.new(customer_id)
 		if (plan = customer.plan)
+			btc = pending.value
 			amount = btc * sell_price.fetch(plan.currency).round(4, :floor)
-			customer.add_btc_credit(txid, btc, amount)
+			customer.add_btc_credit(pending.txid, btc, amount)
 			plan.notify_any_pending_plan!
-			REDIS.hdel("pending_#{CONFIG[:electrum][:currency]}_transactions", txid)
-			txid
+			repo.remove_transaction(pending)
+			pending.txid
 		else
-			warn "No plan for #{customer_id} cannot save #{txid}"
+			warn "No plan for #{customer_id} cannot save #{pending.txid}"
 		end
 	end
-}
+end
 
-puts done.compact.join("\n")
+puts done.join("\n")

lib/electrum.rb 🔗

@@ -41,6 +41,8 @@ class Electrum
 	end
 
 	class Transaction
+		attr_reader :tx_hash
+
 		def initialize(electrum, tx_hash, tx)
 			raise NoTransaction, "No tx for #{@currency} #{tx_hash}" unless tx
 

lib/pending_transaction_repo.rb 🔗

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require "lazy_object"
+
+class PendingTransactionRepo
+	class PendingTransaction
+		def initialize(tx, address)
+			@tx = tx
+			@address = address
+		end
+
+		def tx_hash
+			@tx.tx_hash
+		end
+
+		def txid
+			"#{tx_hash}/#{@address}"
+		end
+
+		def value
+			@tx.amount_for(@address)
+		end
+
+		def outgoing?
+			value <= 0
+		end
+	end
+
+	def initialize(key)
+		@key = key
+	end
+
+	def redis
+		@redis ||= LazyObject.new { REDIS }
+	end
+
+	def electrum
+		@electrum ||= LazyObject.new { ELECTRUM }
+	end
+
+	def error_handler(&block)
+		@error_handler = block
+	end
+
+	def map
+		redis.hgetall(@key).map { |(txid, customer_id)|
+			begin
+				tx_hash, address = txid.split("/", 2)
+
+				txn = electrum.gettransaction(tx_hash)
+
+				yield [PendingTransaction.new(txn, address), customer_id]
+			rescue StandardError => e
+				raise e unless @error_handler.call(e)
+			end
+		}.compact
+	end
+
+	def remove_transaction(pending)
+		redis.hdel(@key, pending.txid)
+	end
+end

test/test_pending_transaction_repo.rb 🔗

@@ -0,0 +1,145 @@
+# frozen_string_literal: true
+
+require "pending_transaction_repo"
+
+class PendingTransactionRepo
+	def setup_mocks
+		@redis = Minitest::Mock.new
+		@electrum = Minitest::Mock.new
+	end
+end
+
+FakeElectrumTransaction = Struct.new(:tx_hash, :confirmations, :value) {
+	def amount_for(_addr)
+		value
+	end
+}
+
+class TestPendingTransactionRepo < Minitest::Test
+	def test_empty_map
+		repo = PendingTransactionRepo.new("key")
+		repo.setup_mocks
+		repo.redis.expect(
+			:hgetall,
+			[],
+			["key"]
+		)
+		repo.map do |_pending, _customer_id|
+			flunk "Shouldn't yield when empty"
+		end
+
+		assert_mock repo.redis
+		assert_mock repo.electrum
+	end
+
+	def test_map
+		repo = PendingTransactionRepo.new("key")
+		repo.setup_mocks
+		repo.redis.expect(
+			:hgetall,
+			[["tx/addr", "1234"]],
+			["key"]
+		)
+		repo.electrum.expect(
+			:gettransaction,
+			FakeElectrumTransaction.new("tx", 6, 0.5),
+			["tx"]
+		)
+
+		v = repo.map { |pending, customer_id|
+			"#{pending.value} #{customer_id}"
+		}
+
+		assert_equal ["0.5 1234"], v, "Should have returned result of block"
+
+		assert_mock repo.redis
+		assert_mock repo.electrum
+	end
+
+	def test_error_handler
+		repo = PendingTransactionRepo.new("key")
+		repo.setup_mocks
+		repo.redis.expect(
+			:hgetall,
+			[["tx/addr", "1234"], ["missing/addr", "1234"]],
+			["key"]
+		)
+		def repo.electrum
+			Class.new {
+				def gettransaction(txid)
+					if txid == "missing"
+						raise Electrum::NoTransaction, "Couldn't find"
+					end
+
+					FakeElectrumTransaction.new("tx", 6, 0.5)
+				end
+			}.new
+		end
+
+		repo.error_handler do |e|
+			case e
+			when Electrum::NoTransaction
+				true
+			end
+		end
+
+		v = repo.map { |pending, customer_id|
+			"#{pending.value} #{customer_id}"
+		}
+
+		assert_equal ["0.5 1234"], v, "Should have returned result of block"
+
+		assert_mock repo.redis
+	end
+
+	def test_other_errors
+		repo = PendingTransactionRepo.new("key")
+		repo.setup_mocks
+		repo.redis.expect(
+			:hgetall,
+			[["tx/addr", "1234"], ["error/addr", "1234"]],
+			["key"]
+		)
+		def repo.electrum
+			Class.new {
+				def gettransaction(txid)
+					raise ArgumentError, "Oh no" if txid == "error"
+
+					FakeElectrumTransaction.new("tx", 6, 0.5)
+				end
+			}.new
+		end
+
+		repo.error_handler do |e|
+			case e
+			when Electrum::NoTransaction
+				true
+			end
+		end
+
+		assert_raises(ArgumentError) do
+			repo.map { |pending, customer_id|
+				"#{pending.value} #{customer_id}"
+			}
+		end
+
+		assert_mock repo.redis
+	end
+
+	def test_remove_transaction
+		repo = PendingTransactionRepo.new("key")
+		repo.setup_mocks
+
+		pending = PendingTransactionRepo::PendingTransaction.new(
+			FakeElectrumTransaction.new("tx", 6, 0.5),
+			"addr"
+		)
+
+		repo.redis.expect(:hdel, nil, ["key", "tx/addr"])
+
+		repo.remove_transaction(pending)
+
+		assert_mock repo.redis
+		assert_mock repo.electrum
+	end
+end