blockonomics / whmcs-bitcoin-plugin

Accept bitcoins on your WHMCS, payments go directly into your wallet
8 stars 12 forks source link

Check for already processed transaction #78

Closed DarrenWestwood closed 4 years ago

DarrenWestwood commented 4 years ago

Fixes duplicate transactions when confirmation is not set to 2

jusasiiv commented 4 years ago

Nice code for fix. But placement for check prevents doing Test Setup as from first test callback onwards there will be txid WarningThisIsAGeneratedTestPaymentAndNotARealBitcoinTransaction in db

We could move https://github.com/blockonomics/whmcs-bitcoin-plugin/pull/78/files#diff-4d9ac2c7462a79fcb731a02afe863586R114

if($txid == 'WarningThisIsAGeneratedTestPaymentAndNotARealBitcoinTransaction') {
    $txid = 'WarningThisIsATestTransaction_' . md5(uniqid(rand(), true));
}

but need to check that using generated test txid in other places wont mess anything

jusasiiv commented 4 years ago

Actually thinking about this, the txid check could be moved to where the WHMCS own checkcbtrans is to keep the functionality the same

DarrenWestwood commented 4 years ago

Actually thinking about this, the txid check could be moved to where the WHMCS own checkcbtrans is to keep the functionality the same

Moved in f78fbc1

jusasiiv commented 4 years ago

And lastly we should remove

if($txid == 'WarningThisIsAGeneratedTestPaymentAndNotARealBitcoinTransaction') {
    $txid = 'WarningThisIsATestTransaction_' . md5(uniqid(rand(), true));
}

as now it causes the double transaction bug when making test transactions

DarrenWestwood commented 4 years ago

And lastly we should remove

Updated in 375d106

jusasiiv commented 4 years ago

Changed the way the txid is checked 2606792b1e1ef86fb7d78ce7898e682db4093d3f

DarrenWestwood commented 4 years ago

Changed the way the txid is checked 2606792

Tested. The change works as desired without any issues.