[Logic error] All transactions from a payment must be confirmed before setting the payment as :confirmed

[maesitos]

Since BitcoinPayable::Interactors::TransactionProcessor::DeterminePaymentStatus checks a single transaction and sets the payment as :confirmed if only that particular transaction has more confirmations than BitcoinPayable.config.confirmations there is an immediate loophole:

• I create a payment of 10BTC.
• I pay just 0.1BTC
• Sooner or lather the 0.1BTC transaction will have more than BitcoinPayable.config.confirmations
• The whole payment will be set as:confirmed without it being first :fully_paid or receiving the correct number of coins.

It's true AASM restricts the states the class can transition from

event :confirm do
        transitions :from => [:pending, :paid_in_full], :to => :confirmed
end

Nonetheless the current code still allows the class to transition from :pending to :confirm and I don't recommend to limit the transition from only:paid_in_full

[maesitos]

The pull request https://github.com/Sailias/bitcoin_payable/pull/41 solves the problem and gives more clarity to the code.

[maesitos]

Actually since BitcoinPayable::Interactors::TransactionProcessor::DeterminePaymentStatus only sets the payment as confirmed if the total fiat received is correct the steps to bypass the payment system is:

• Create a 10BTC payment
• Pay 0.1BTC
• Wait for BitcoinPayable.config.confirmations - 1 confirmations
• Double spend 9.9 BTC or send 9.9BTC that will never reach BitcoinPayable.config.confirmations
• The next block will set the 0.1BTC transaction as having exactly BitcoinPayable.config.confirmations making the whole payment as confirmed while the 9.9BTC will never reach BitcoinPayable.config.confirmations confirmations.

It's obviously a pretty involved process and a very hard attack but the user of the Gem can receive a double spend attack when he thought he was waiting for n confirmations.

[Sailias]

@maesitos I see. It should ensure each transaction meets the minimum confirmation count.

[maesitos]

@Sailias PR #41 is a simple fix to this problem.