commit ac77e1c832e43ec95665ffe6bbb9450c70495a3c
parent 0d1d6348442532f01a09781419e6a29c8e645ab5
Author: Antoine A <>
Date: Tue, 17 Oct 2023 18:48:18 +0000
Fix withdrawal not being registered as incoming transaction
Diffstat:
9 files changed, 81 insertions(+), 64 deletions(-)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/BankIntegrationApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/BankIntegrationApi.kt
@@ -63,14 +63,17 @@ fun Routing.bankIntegrationApi(db: Database, ctx: BankApplicationContext) {
val wopid = call.expectUriComponent("wopid")
val req = call.receive<BankWithdrawalOperationPostRequest>()
val op = getWithdrawal(db, wopid) // throws 404 if not found.
+
+ // TODO move logic into DB
if (op.selectionDone) { // idempotency
if (op.selectedExchangePayto != req.selected_exchange && op.reservePub != req.reserve_pub) throw conflict(
hint = "Cannot select different exchange and reserve pub. under the same withdrawal operation",
talerEc = TalerErrorCode.TALER_EC_BANK_WITHDRAWAL_OPERATION_RESERVE_SELECTION_CONFLICT
)
}
+ // TODO check account is exchange
val dbSuccess: Boolean = if (!op.selectionDone) { // Check if reserve pub. was used in _another_ withdrawal.
- if (db.bankTransactionCheckExists(req.reserve_pub)) throw conflict(
+ if (db.checkReservePubReuse(req.reserve_pub)) throw conflict(
"Reserve pub. already used", TalerErrorCode.TALER_EC_BANK_DUPLICATE_RESERVE_PUB_SUBJECT
)
db.talerWithdrawalSetDetails(
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
@@ -847,20 +847,14 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
}
}
- /**
- * Only checks if a bank transaction with the given subject
- * exists. That's only used in the /admin/add-incoming, to
- * prevent a public key from being reused.
- *
- * Returns the row ID if found, null otherwise.
- */
- suspend fun bankTransactionCheckExists(subject: String): Boolean = conn { conn ->
+ suspend fun checkReservePubReuse(reservePub: EddsaPublicKey): Boolean = conn { conn ->
val stmt = conn.prepareStatement("""
- SELECT 1
- FROM bank_account_transactions
- WHERE subject = ?;
+ SELECT 1 FROM taler_exchange_incoming WHERE reserve_pub = ?
+ UNION ALL
+ SELECT 1 FROM taler_withdrawal_operations WHERE reserve_pub = ?
""")
- stmt.setString(1, subject)
+ stmt.setBytes(1, reservePub.raw)
+ stmt.setBytes(2, reservePub.raw)
stmt.oneOrNull { } != null
}
@@ -1145,7 +1139,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
walletBankAccount = it.getLong("wallet_bank_account"),
confirmationDone = it.getBoolean("confirmation_done"),
aborted = it.getBoolean("aborted"),
- reservePub = it.getString("reserve_pub"),
+ reservePub = it.getBytes("reserve_pub")?.run(::EddsaPublicKey),
withdrawalUuid = it.getObject("withdrawal_uuid") as UUID
)
}
@@ -1177,17 +1171,19 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
suspend fun talerWithdrawalSetDetails(
opUuid: UUID,
exchangePayto: IbanPayTo,
- reservePub: String
+ reservePub: EddsaPublicKey
): Boolean = conn { conn ->
+ val subject = IncomingTxMetadata(reservePub).encode()
val stmt = conn.prepareStatement("""
UPDATE taler_withdrawal_operations
- SET selected_exchange_payto = ?, reserve_pub = ?, selection_done = true
+ SET selected_exchange_payto = ?, reserve_pub = ?, subject = ?, selection_done = true
WHERE withdrawal_uuid=?
"""
)
stmt.setString(1, exchangePayto.canonical)
- stmt.setString(2, reservePub)
- stmt.setObject(3, opUuid)
+ stmt.setBytes(2, reservePub.raw)
+ stmt.setString(3, subject)
+ stmt.setObject(4, opUuid)
stmt.executeUpdateViolation()
}
@@ -1527,7 +1523,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
pmtInfId: String = "not used",
endToEndId: String = "not used",
): TalerAddIncomingCreationResult = conn { conn ->
- val subject = IncomingTxMetadata(req.reserve_pub).encode()
+ val subject = IncomingTxMetadata(req.reserve_pub).encode()
val stmt = conn.prepareStatement("""
SELECT
out_creditor_not_found
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt b/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt
@@ -269,7 +269,7 @@ data class TalerWithdrawalOperation(
val selectionDone: Boolean = false,
val aborted: Boolean = false,
val confirmationDone: Boolean = false,
- val reservePub: String?,
+ val reservePub: EddsaPublicKey?,
val selectedExchangePayto: IbanPayTo?,
val walletBankAccount: Long
)
@@ -399,7 +399,7 @@ data class BankAccountGetWithdrawalResponse(
val aborted: Boolean,
val confirmation_done: Boolean,
val selection_done: Boolean,
- val selected_reserve_pub: String? = null,
+ val selected_reserve_pub: EddsaPublicKey? = null,
val selected_exchange_account: IbanPayTo? = null
)
@@ -467,7 +467,7 @@ data class BankWithdrawalOperationStatus(
*/
@Serializable
data class BankWithdrawalOperationPostRequest(
- val reserve_pub: String,
+ val reserve_pub: EddsaPublicKey,
val selected_exchange: IbanPayTo,
)
diff --git a/bank/src/test/kotlin/BankIntegrationApiTest.kt b/bank/src/test/kotlin/BankIntegrationApiTest.kt
@@ -30,7 +30,7 @@ class BankIntegrationApiTest {
val r = client.post("/taler-integration/withdrawal-operation/${uuid}") {
jsonBody(BankWithdrawalOperationPostRequest(
- reserve_pub = "RESERVE-FOO",
+ reserve_pub = randEddsaPublicKey(),
selected_exchange = IbanPayTo("payto://iban/ABC123")
))
}.assertOk()
@@ -64,7 +64,7 @@ class BankIntegrationApiTest {
))
val op = db.talerWithdrawalGet(uuid)
assert(op?.aborted == false)
- assert(db.talerWithdrawalSetDetails(uuid, IbanPayTo("payto://iban/exchange-payto"), "reserve_pub"))
+ assert(db.talerWithdrawalSetDetails(uuid, IbanPayTo("payto://iban/exchange-payto"), randEddsaPublicKey()))
client.post("/withdrawals/${uuid}/abort") {
basicAuth("merchant", "merchant-password")
@@ -103,7 +103,7 @@ class BankIntegrationApiTest {
assert(db.talerWithdrawalSetDetails(
opUuid = uuid,
exchangePayto = IbanPayTo("payto://iban/EXCHANGE-IBAN-XYZ"),
- reservePub = "UNCHECKED-RESERVE-PUB"
+ reservePub = randEddsaPublicKey()
))
// Starting the bank and POSTing as Foo to /confirm the operation.
diff --git a/bank/src/test/kotlin/DatabaseTest.kt b/bank/src/test/kotlin/DatabaseTest.kt
@@ -291,8 +291,8 @@ class DatabaseTest {
// Setting the details.
assert(db.talerWithdrawalSetDetails(
opUuid = uuid,
- exchangePayto = IbanPayTo("payto://iban/BAR-IBAN-ABC"),
- reservePub = "UNCHECKED-RESERVE-PUB"
+ exchangePayto = IbanPayTo("payto://iban/FOO-IBAN-XYZ"),
+ reservePub = randEddsaPublicKey()
))
val opSelected = db.talerWithdrawalGet(uuid)
assert(opSelected?.selectionDone == true && !opSelected.confirmationDone)
diff --git a/bank/src/test/kotlin/WireGatewayApiTest.kt b/bank/src/test/kotlin/WireGatewayApiTest.kt
@@ -238,12 +238,27 @@ class WireGatewayApiTest {
db.bankTransactionCreate(genTx("bogus foobar")).assertSuccess()
// Bar pays Foo once, but that should not appear in the result.
db.bankTransactionCreate(genTx("payout", creditorId = 1, debtorId = 2)).assertSuccess()
- // Gen two transactions using row bank transaction logic
- repeat(2) {
- db.bankTransactionCreate(
- genTx(IncomingTxMetadata(randShortHashCode()).encode(), 2, 1)
- ).assertSuccess()
+ // Gen one transaction using raw bank transaction logic
+ db.bankTransactionCreate(
+ genTx(IncomingTxMetadata(randShortHashCode()).encode(), 2, 1)
+ ).assertSuccess()
+ // Gen one transaction using withdraw logic
+ val uuid = client.post("/accounts/merchant/withdrawals") {
+ basicAuth("merchant", "merchant-password")
+ jsonBody(json { "amount" to "KUDOS:9" })
+ }.assertOk().run {
+ val resp = Json.decodeFromString<BankAccountCreateWithdrawalResponse>(bodyAsText())
+ resp.taler_withdraw_uri.split("/").last()
}
+ client.post("/taler-integration/withdrawal-operation/${uuid}") {
+ jsonBody(json {
+ "reserve_pub" to randEddsaPublicKey()
+ "selected_exchange" to IbanPayTo("payto://iban/EXCHANGE-IBAN-XYZ")
+ })
+ }.assertOk()
+ client.post("/withdrawals/${uuid}/confirm") {
+ basicAuth("merchant", "merchant-password")
+ }.assertOk()
// Check ignore bogus subject
client.get("/accounts/exchange/taler-wire-gateway/history/incoming?delta=7") {
@@ -373,7 +388,7 @@ class WireGatewayApiTest {
db.bankTransactionCreate(genTx("bogus foobar", 1, 2)).assertSuccess()
// Foo pays Bar once, but that should not appear in the result.
db.bankTransactionCreate(genTx("payout")).assertSuccess()
- // Gen two transactions using row bank transaction logic
+ // Gen two transactions using raw bank transaction logic
repeat(2) {
db.bankTransactionCreate(
genTx(OutgoingTxMetadata(randShortHashCode(), ExchangeUrl("http://exchange.example.com/")).encode(), 1, 2)
diff --git a/bank/src/test/kotlin/helpers.kt b/bank/src/test/kotlin/helpers.kt
@@ -171,14 +171,6 @@ fun randBase32Crockford(lenght: Int): String {
return Base32Crockford.encode(bytes)
}
-fun randHashCode(): HashCode {
- return HashCode(randBase32Crockford(64))
-}
-
-fun randShortHashCode(): ShortHashCode {
- return ShortHashCode(randBase32Crockford(32))
-}
-
-fun randEddsaPublicKey(): EddsaPublicKey {
- return EddsaPublicKey(randBase32Crockford(32))
-}
-\ No newline at end of file
+fun randHashCode(): HashCode = HashCode(randBase32Crockford(64))
+fun randShortHashCode(): ShortHashCode = ShortHashCode(randBase32Crockford(32))
+fun randEddsaPublicKey(): EddsaPublicKey = EddsaPublicKey(randBase32Crockford(32))
+\ No newline at end of file
diff --git a/database-versioning/libeufin-bank-0001.sql b/database-versioning/libeufin-bank-0001.sql
@@ -385,7 +385,8 @@ CREATE TABLE IF NOT EXISTS taler_withdrawal_operations
,selection_done BOOLEAN DEFAULT FALSE NOT NULL
,aborted BOOLEAN DEFAULT FALSE NOT NULL
,confirmation_done BOOLEAN DEFAULT FALSE NOT NULL
- ,reserve_pub TEXT NULL -- Kotlin must check it's valid.
+ ,reserve_pub BYTEA UNIQUE CHECK (LENGTH(reserve_pub)=32)
+ ,subject TEXT
,selected_exchange_payto TEXT
,wallet_bank_account BIGINT NOT NULL
REFERENCES bank_accounts(bank_account_id)
diff --git a/database-versioning/procedures.sql b/database-versioning/procedures.sql
@@ -348,12 +348,10 @@ DECLARE
exchange_bank_account_id BIGINT;
sender_bank_account_id BIGINT;
BEGIN
--- Check for idempotence and conflict
-SELECT true
- FROM taler_exchange_incoming
- JOIN bank_account_transactions AS txs
- ON bank_transaction=txs.bank_transaction_id
- WHERE reserve_pub = in_reserve_pub
+-- Check conflict
+SELECT true FROM taler_exchange_incoming WHERE reserve_pub = in_reserve_pub
+UNION ALL
+SELECT true FROM taler_withdrawal_operations WHERE reserve_pub = in_reserve_pub
INTO out_reserve_pub_reuse;
IF out_reserve_pub_reuse THEN
RETURN;
@@ -590,6 +588,7 @@ CREATE OR REPLACE FUNCTION confirm_taler_withdrawal(
IN in_end_to_end_id TEXT,
OUT out_no_op BOOLEAN,
OUT out_balance_insufficient BOOLEAN,
+ OUT out_creditor_not_found BOOLEAN,
OUT out_exchange_not_found BOOLEAN,
OUT out_already_confirmed_conflict BOOLEAN
)
@@ -597,22 +596,23 @@ LANGUAGE plpgsql
AS $$
DECLARE
confirmation_done_local BOOLEAN;
- reserve_pub_local TEXT;
+ subject_local TEXT;
+ reserve_pub_local BYTEA;
selected_exchange_payto_local TEXT;
wallet_bank_account_local BIGINT;
amount_local taler_amount;
exchange_bank_account_id BIGINT;
- maybe_balance_insufficient BOOLEAN;
+ tx_row_id BIGINT;
BEGIN
SELECT -- Really no-star policy and instead DECLARE almost one var per column?
confirmation_done,
- reserve_pub,
+ reserve_pub, subject,
selected_exchange_payto,
wallet_bank_account,
(amount).val, (amount).frac
INTO
confirmation_done_local,
- reserve_pub_local,
+ reserve_pub_local, subject_local,
selected_exchange_payto_local,
wallet_bank_account_local,
amount_local.val, amount_local.frac
@@ -647,24 +647,34 @@ THEN
END IF;
out_exchange_not_found=FALSE;
SELECT -- not checking for accounts existence, as it was done above.
- transfer.out_balance_insufficient
- INTO
- out_balance_insufficient
+ transfer.out_balance_insufficient,
+ out_credit_row_id
+ INTO out_balance_insufficient, tx_row_id
FROM bank_wire_transfer(
exchange_bank_account_id,
wallet_bank_account_local,
- reserve_pub_local,
+ subject_local,
amount_local,
in_confirmation_date,
in_acct_svcr_ref,
in_pmt_inf_id,
in_end_to_end_id
) as transfer;
-IF (maybe_balance_insufficient)
-THEN
- out_balance_insufficient=TRUE;
+IF out_balance_insufficient THEN
+ RETURN;
END IF;
-out_balance_insufficient=FALSE;
+
+-- Register incoming transaction
+INSERT
+ INTO taler_exchange_incoming (
+ reserve_pub,
+ bank_transaction
+) VALUES (
+ reserve_pub_local,
+ tx_row_id
+);
+-- notify new transaction
+PERFORM pg_notify('incoming_tx', exchange_bank_account_id || ' ' || tx_row_id);
END $$;
COMMENT ON FUNCTION confirm_taler_withdrawal(uuid, bigint, text, text, text)
IS 'Set a withdrawal operation as confirmed and wire the funds to the exchange.';
