Improve nexus SQL schema and logic

7 files changed, 70 insertions, 78 deletions

diff --git a/database-versioning/libeufin-nexus-0001.sql b/database-versioning/libeufin-nexus-0001.sql
index 6e32e0b1..aae5d065 100644
--- a/database-versioning/libeufin-nexus-0001.sql
+++ b/database-versioning/libeufin-nexus-0001.sql
@@ -21,10 +21,7 @@ CREATE SCHEMA libeufin_nexus;
 SET search_path TO libeufin_nexus;
 
 CREATE TYPE taler_amount
-  AS
-  (val INT8
-  ,frac INT4
-  );
+  AS (val INT8, frac INT4);
 COMMENT ON TYPE taler_amount
   IS 'Stores an amount, fraction is in units of 1/100000000 of the base value';
 
@@ -50,8 +47,10 @@ CREATE TABLE IF NOT EXISTS incoming_transactions
   ,wire_transfer_subject TEXT NOT NULL
   ,execution_time INT8 NOT NULL
   ,debit_payto_uri TEXT NOT NULL
-  ,bank_transfer_id TEXT NOT NULL UNIQUE -- EBICS or Depolymerizer (generic)
+  ,bank_id TEXT NOT NULL UNIQUE
   );
+COMMENT ON COLUMN incoming_transactions.bank_id
+  IS 'ISO20022 AccountServicerReference';
 
 -- only active in exchange mode. Note: duplicate keys are another reason to bounce.
 CREATE TABLE IF NOT EXISTS talerable_incoming_transactions
@@ -65,8 +64,10 @@ CREATE TABLE IF NOT EXISTS outgoing_transactions
   ,wire_transfer_subject TEXT
   ,execution_time INT8 NOT NULL
   ,credit_payto_uri TEXT
-  ,bank_transfer_id TEXT NOT NULL UNIQUE
+  ,message_id TEXT NOT NULL UNIQUE
   );
+COMMENT ON COLUMN outgoing_transactions.message_id
+  IS 'ISO20022 MessageIdentification';
 
 CREATE TABLE IF NOT EXISTS initiated_outgoing_transactions
   (initiated_outgoing_transaction_id INT8 GENERATED BY DEFAULT AS IDENTITY UNIQUE
@@ -82,7 +83,6 @@ CREATE TABLE IF NOT EXISTS initiated_outgoing_transactions
   ,request_uid TEXT NOT NULL UNIQUE CHECK (char_length(request_uid) <= 35)
   ,failure_message TEXT -- NOTE: that may mix soon failures (those found at initiation time), or late failures (those found out along a fetch operation)
   );
-
 COMMENT ON COLUMN initiated_outgoing_transactions.outgoing_transaction_id
   IS 'Points to the bank transaction that was found via nexus-fetch.  If "submitted" is false or nexus-fetch could not download this initiation, this column is expected to be NULL.';
 COMMENT ON COLUMN initiated_outgoing_transactions.request_uid
diff --git a/database-versioning/libeufin-nexus-procedures.sql b/database-versioning/libeufin-nexus-procedures.sql
index 280816c1..8e469055 100644
--- a/database-versioning/libeufin-nexus-procedures.sql
+++ b/database-versioning/libeufin-nexus-procedures.sql
@@ -30,21 +30,21 @@ $do$;
 CREATE FUNCTION register_outgoing(
   IN in_amount taler_amount
   ,IN in_wire_transfer_subject TEXT
-  ,IN in_execution_time BIGINT
+  ,IN in_execution_time INT8
   ,IN in_credit_payto_uri TEXT
-  ,IN in_bank_transfer_id TEXT
-  ,OUT out_tx_id BIGINT
+  ,IN in_message_id TEXT
+  ,OUT out_tx_id INT8
   ,OUT out_found BOOLEAN
   ,OUT out_initiated BOOLEAN
 )
 LANGUAGE plpgsql AS $$
 DECLARE
-init_id BIGINT;
+init_id INT8;
 BEGIN
 -- Check if already registered
 SELECT outgoing_transaction_id INTO out_tx_id
   FROM outgoing_transactions
-  WHERE bank_transfer_id = in_bank_transfer_id;
+  WHERE message_id = in_message_id;
 IF FOUND THEN
   out_found = true;
   -- TODO Should we update the subject and credit payto if it's finally found
@@ -59,21 +59,23 @@ ELSE
     ,wire_transfer_subject
     ,execution_time
     ,credit_payto_uri
-    ,bank_transfer_id
+    ,message_id
   ) VALUES (
     in_amount
     ,in_wire_transfer_subject
     ,in_execution_time
     ,in_credit_payto_uri
-    ,in_bank_transfer_id
+    ,in_message_id
   )
     RETURNING outgoing_transaction_id
       INTO out_tx_id;
 
   -- Reconciles the related initiated transaction
   UPDATE initiated_outgoing_transactions
-    SET outgoing_transaction_id = out_tx_id
-    WHERE request_uid = in_bank_transfer_id
+    SET 
+      outgoing_transaction_id = out_tx_id
+      ,submitted = 'success'
+    WHERE request_uid = in_message_id
     RETURNING true INTO out_initiated;
 END IF;
 END $$;
@@ -83,18 +85,18 @@ COMMENT ON FUNCTION register_outgoing
 CREATE FUNCTION register_incoming(
   IN in_amount taler_amount
   ,IN in_wire_transfer_subject TEXT
-  ,IN in_execution_time BIGINT
+  ,IN in_execution_time INT8
   ,IN in_debit_payto_uri TEXT
-  ,IN in_bank_transfer_id TEXT
+  ,IN in_bank_id TEXT
   ,OUT out_found BOOLEAN
-  ,OUT out_tx_id BIGINT
+  ,OUT out_tx_id INT8
 )
 LANGUAGE plpgsql AS $$
 BEGIN
 -- Check if already registered
 SELECT incoming_transaction_id INTO out_tx_id
   FROM incoming_transactions
-  WHERE bank_transfer_id = in_bank_transfer_id;
+  WHERE bank_id = in_bank_id;
 IF FOUND THEN
   out_found = true;
   -- TODO Should we check that amount and other info match ?
@@ -105,13 +107,13 @@ ELSE
     ,wire_transfer_subject
     ,execution_time
     ,debit_payto_uri
-    ,bank_transfer_id
+    ,bank_id
   ) VALUES (
     in_amount
     ,in_wire_transfer_subject
     ,in_execution_time
     ,in_debit_payto_uri
-    ,in_bank_transfer_id
+    ,in_bank_id
   ) RETURNING incoming_transaction_id INTO out_tx_id;
 END IF;
 END $$;
@@ -119,20 +121,20 @@ COMMENT ON FUNCTION register_incoming
   IS 'Register an incoming transaction';
 
 CREATE FUNCTION bounce_incoming(
-  IN tx_id BIGINT
+  IN tx_id INT8
   ,IN in_bounce_amount taler_amount
-  ,IN in_now_date BIGINT
+  ,IN in_now_date INT8
   ,OUT out_bounce_id TEXT
 )
 LANGUAGE plpgsql AS $$
 DECLARE
-bank_id TEXT;
+local_bank_id TEXT;
 payto_uri TEXT;
-init_id BIGINT;
+init_id INT8;
 BEGIN
 -- Get incoming transaction bank ID and creditor
-SELECT bank_transfer_id, debit_payto_uri 
-  INTO bank_id, payto_uri
+SELECT bank_id, debit_payto_uri 
+  INTO local_bank_id, payto_uri
   FROM incoming_transactions
   WHERE incoming_transaction_id = tx_id;
 -- Generate a bounce ID deterministically from the bank ID
@@ -171,22 +173,22 @@ COMMENT ON FUNCTION bounce_incoming
 CREATE FUNCTION register_incoming_and_bounce(
   IN in_amount taler_amount
   ,IN in_wire_transfer_subject TEXT
-  ,IN in_execution_time BIGINT
+  ,IN in_execution_time INT8
   ,IN in_debit_payto_uri TEXT
-  ,IN in_bank_transfer_id TEXT
+  ,IN in_bank_id TEXT
   ,IN in_bounce_amount taler_amount
-  ,IN in_now_date BIGINT
+  ,IN in_now_date INT8
   ,OUT out_found BOOLEAN
-  ,OUT out_tx_id BIGINT
+  ,OUT out_tx_id INT8
   ,OUT out_bounce_id TEXT
 )
 LANGUAGE plpgsql AS $$
 DECLARE
-init_id BIGINT;
+init_id INT8;
 BEGIN
 -- Register the incoming transaction
 SELECT reg.out_found, reg.out_tx_id
-  FROM register_incoming(in_amount, in_wire_transfer_subject, in_execution_time, in_debit_payto_uri, in_bank_transfer_id) as reg
+  FROM register_incoming(in_amount, in_wire_transfer_subject, in_execution_time, in_debit_payto_uri, in_bank_id) as reg
   INTO out_found, out_tx_id;
 
 -- Bounce the incoming transaction
@@ -198,18 +200,18 @@ COMMENT ON FUNCTION register_incoming_and_bounce
 CREATE FUNCTION register_incoming_and_talerable(
   IN in_amount taler_amount
   ,IN in_wire_transfer_subject TEXT
-  ,IN in_execution_time BIGINT
+  ,IN in_execution_time INT8
   ,IN in_debit_payto_uri TEXT
-  ,IN in_bank_transfer_id TEXT
+  ,IN in_bank_id TEXT
   ,IN in_reserve_public_key BYTEA
   ,OUT out_found BOOLEAN
-  ,OUT out_tx_id BIGINT
+  ,OUT out_tx_id INT8
 )
 LANGUAGE plpgsql AS $$
 BEGIN
 -- Register the incoming transaction
 SELECT reg.out_found, reg.out_tx_id
-  FROM register_incoming(in_amount, in_wire_transfer_subject, in_execution_time, in_debit_payto_uri, in_bank_transfer_id) as reg
+  FROM register_incoming(in_amount, in_wire_transfer_subject, in_execution_time, in_debit_payto_uri, in_bank_id) as reg
   INTO out_found, out_tx_id;
 
 -- Register as talerable bounce
diff --git a/integration/src/test/kotlin/IntegrationTest.kt b/integration/src/test/kotlin/IntegrationTest.kt
index 0b483e41..a1fb79b6 100644
--- a/integration/src/test/kotlin/IntegrationTest.kt
+++ b/integration/src/test/kotlin/IntegrationTest.kt
@@ -155,7 +155,7 @@ class IntegrationTest {
                 debitPaytoUri = userPayTo.canonical,
                 wireTransferSubject = "Error test ${Base32Crockford.encode(reservePub)}",
                 executionTime = Instant.now(),
-                bankTransferId = "error"
+                bankId = "error"
             )
 
             assertException("ERROR: cashin failed: missing exchange account") {
@@ -214,7 +214,7 @@ class IntegrationTest {
                 debitPaytoUri = userPayTo.canonical,
                 wireTransferSubject = "Success ${Base32Crockford.encode(randBytes(32))}",
                 executionTime = Instant.now(),
-                bankTransferId = "success"
+                bankId = "success"
             ))
             checkCount(db, 2, 1, 1)
             client.get("http://0.0.0.0:8080/accounts/exchange/transactions") {
@@ -287,7 +287,7 @@ class IntegrationTest {
                         debitPaytoUri = userPayTo.canonical,
                         wireTransferSubject = subject,
                         executionTime = Instant.now(),
-                        bankTransferId = Base32Crockford.encode(reservePub)
+                        bankId = Base32Crockford.encode(reservePub)
                     )
                 )
                 val converted = client.get("http://0.0.0.0:8080/conversion-info/cashin-rate?amount_debit=EUR:${20 + i}")
diff --git a/nexus/src/main/kotlin/tech/libeufin/nexus/Database.kt b/nexus/src/main/kotlin/tech/libeufin/nexus/Database.kt
index 3d7c9677..b0bda010 100644
--- a/nexus/src/main/kotlin/tech/libeufin/nexus/Database.kt
+++ b/nexus/src/main/kotlin/tech/libeufin/nexus/Database.kt
@@ -50,10 +50,11 @@ data class IncomingPayment(
     val wireTransferSubject: String,
     val debitPaytoUri: String,
     val executionTime: Instant,
-    val bankTransferId: String
+    /** ISO20022 AccountServicerReference */
+    val bankId: String
 )  {
     override fun toString(): String {
-        return "IN ${executionTime.fmtDate()} '$amount $bankTransferId' debitor=$debitPaytoUri subject=$wireTransferSubject"
+        return "IN ${executionTime.fmtDate()} $amount '$bankId' debitor=$debitPaytoUri subject=$wireTransferSubject"
     }
 }
 
@@ -128,12 +129,13 @@ enum class PaymentInitiationOutcome {
 data class OutgoingPayment(
     val amount: TalerAmount,
     val executionTime: Instant,
-    val bankTransferId: String,
+    /** ISO20022 MessageIdentification */
+    val messageId: String,
     val creditPaytoUri: String? = null, // not showing in camt.054
     val wireTransferSubject: String? = null // not showing in camt.054
 ) {
     override fun toString(): String {
-        return "OUT ${executionTime.fmtDate()} $amount '$bankTransferId' creditor=$creditPaytoUri subject=$wireTransferSubject"
+        return "OUT ${executionTime.fmtDate()} $amount '$messageId' creditor=$creditPaytoUri subject=$wireTransferSubject"
     }
 }
 
@@ -239,7 +241,7 @@ class Database(dbConfig: String): java.io.Closeable {
         stmt.setString(3, paymentData.wireTransferSubject)
         stmt.setLong(4, executionTime)
         stmt.setString(5, paymentData.creditPaytoUri)
-        stmt.setString(6, paymentData.bankTransferId)
+        stmt.setString(6, paymentData.messageId)
 
         stmt.executeQuery().use {
             when {
@@ -291,7 +293,7 @@ class Database(dbConfig: String): java.io.Closeable {
         stmt.setString(3, paymentData.wireTransferSubject)
         stmt.setLong(4, executionTime)
         stmt.setString(5, paymentData.debitPaytoUri)
-        stmt.setString(6, paymentData.bankTransferId)
+        stmt.setString(6, paymentData.bankId)
         stmt.setLong(7, bounceAmount.value)
         stmt.setInt(8, bounceAmount.fraction)
         stmt.setLong(9, refundTimestamp)
@@ -336,7 +338,7 @@ class Database(dbConfig: String): java.io.Closeable {
         stmt.setString(3, paymentData.wireTransferSubject)
         stmt.setLong(4, executionTime)
         stmt.setString(5, paymentData.debitPaytoUri)
-        stmt.setString(6, paymentData.bankTransferId)
+        stmt.setString(6, paymentData.bankId)
         stmt.setBytes(7, reservePub)
         stmt.executeQuery().use {
             when {
diff --git a/nexus/src/main/kotlin/tech/libeufin/nexus/EbicsFetch.kt b/nexus/src/main/kotlin/tech/libeufin/nexus/EbicsFetch.kt
index 5f51d023..d4feaf98 100644
--- a/nexus/src/main/kotlin/tech/libeufin/nexus/EbicsFetch.kt
+++ b/nexus/src/main/kotlin/tech/libeufin/nexus/EbicsFetch.kt
@@ -262,9 +262,8 @@ private suspend fun getTalerReservePub(
 }
 
 /**
- * Ingests any outgoing payment that was NOT ingested yet.  It
- * links it to the initiated outgoing transaction that originated
- * it.
+ * Ingests an outgoing payment. It links it to the initiated
+ * outgoing transaction that originated it.
  *
  * @param db database handle.
  * @param payment payment to (maybe) ingest.
@@ -280,14 +279,13 @@ suspend fun ingestOutgoingPayment(
         else 
             logger.debug("$payment recovered")
     } else {
-        logger.debug("OUT '${payment.bankTransferId}' already seen")
+        logger.debug("OUT '${payment.messageId}' already seen")
     }
 }
 
 /**
- * Ingests any incoming payment that was NOT ingested yet.  Stores
- * the payment into valid talerable ones or bounces it, according
- * to the subject.
+ * Ingests an incoming payment.  Stores the payment into valid talerable ones
+ * or bounces it, according to the subject.
  *
  * @param db database handle.
  * @param currency fiat currency of the watched bank account.
@@ -299,7 +297,7 @@ suspend fun ingestIncomingPayment(
 ) {
     val reservePub = getTalerReservePub(payment)
     if (reservePub == null) {
-        logger.debug("Incoming payment with UID '${payment.bankTransferId}'" +
+        logger.debug("Incoming payment with UID '${payment.bankId}'" +
                 " has invalid subject: ${payment.wireTransferSubject}."
         )
         val result = db.registerMalformedIncoming(
@@ -310,14 +308,14 @@ suspend fun ingestIncomingPayment(
         if (result.new) {
             logger.debug("$payment bounced in '${result.bounceId}'")
         } else {
-            logger.debug("IN '${payment.bankTransferId}' already seen and bounced in '${result.bounceId}'")
+            logger.debug("IN '${payment.bankId}' already seen and bounced in '${result.bounceId}'")
         }
     } else {
         val result = db.registerTalerableIncoming(payment, reservePub)
         if (result.new) {
             logger.debug("$payment")
         } else {
-            logger.debug("IN '${payment.bankTransferId}' already seen")
+            logger.debug("IN '${payment.bankId}' already seen")
         }
     }
 }
diff --git a/nexus/src/main/kotlin/tech/libeufin/nexus/Iso20022.kt b/nexus/src/main/kotlin/tech/libeufin/nexus/Iso20022.kt
index 35fbec8d..239cdcfa 100644
--- a/nexus/src/main/kotlin/tech/libeufin/nexus/Iso20022.kt
+++ b/nexus/src/main/kotlin/tech/libeufin/nexus/Iso20022.kt
@@ -318,8 +318,7 @@ fun parseTxNotif(
         }
         when (kind) {
             "CRDT" -> {
-                // Obtaining payment UID.
-                val uidFromBank: String = requireUniqueChildNamed("Refs") {
+                val bankId: String = requireUniqueChildNamed("Refs") {
                     requireUniqueChildNamed("AcctSvcrRef") {
                         focusElement.textContent
                     }
@@ -334,7 +333,7 @@ fun parseTxNotif(
                     subject
                 }
                 if (subject == null) {
-                    logger.debug("Skip notification $uidFromBank, missing subject")
+                    logger.debug("Skip notification '$bankId', missing subject")
                     return@notificationForEachTx
                 }
 
@@ -361,7 +360,7 @@ fun parseTxNotif(
                 incoming.add(
                     IncomingPayment(
                         amount = amount,
-                        bankTransferId = uidFromBank,
+                        bankId = bankId,
                         debitPaytoUri = debtorPayto.toString(),
                         executionTime = bookDate,
                         wireTransferSubject = subject.toString()
@@ -369,25 +368,16 @@ fun parseTxNotif(
                 )
             }
             "DBIT" -> {
-                /**
-                 * The MsgId extracted in the block below matches the one that
-                 * was specified as the MsgId element in the pain.001 that originated
-                 * this outgoing payment.  MsgId is considered unique because the
-                 * bank enforces its uniqueness.  Associating MsgId to this outgoing
-                 * payment is also convenient to match its initiated outgoing payment
-                 * in the database for reconciliation.
-                 */
-                val uidFromBank = StringBuilder()
-                requireUniqueChildNamed("Refs") {
+                val messageId = requireUniqueChildNamed("Refs") {
                     requireUniqueChildNamed("MsgId") {
-                        uidFromBank.append(focusElement.textContent)
+                        focusElement.textContent
                     }
                 }
 
                 outgoing.add(
                     OutgoingPayment(
                         amount = amount,
-                        bankTransferId = uidFromBank.toString(),
+                        messageId = messageId,
                         executionTime = bookDate
                     )
                 )
diff --git a/nexus/src/test/kotlin/Common.kt b/nexus/src/test/kotlin/Common.kt
index d35315d6..cbe281d5 100644
--- a/nexus/src/test/kotlin/Common.kt
+++ b/nexus/src/test/kotlin/Common.kt
@@ -97,15 +97,15 @@ fun genInPay(subject: String) =
         debitPaytoUri = "payto://iban/not-used",
         wireTransferSubject = subject,
         executionTime = Instant.now(),
-        bankTransferId = "entropic"
+        bankId = "entropic"
     )
 
-// Generates an outgoing payment, given its subject.
-fun genOutPay(subject: String, bankTransferId: String) =
+// Generates an outgoing payment, given its subject and messageId
+fun genOutPay(subject: String, messageId: String) =
     OutgoingPayment(
         amount = TalerAmount(44, 0, "KUDOS"),
         creditPaytoUri = "payto://iban/TEST-IBAN?receiver-name=Test",
         wireTransferSubject = subject,
         executionTime = Instant.now(),
-        bankTransferId = bankTransferId
+        messageId = messageId
     )
\ No newline at end of file