Use bytea for Crockford's Base32 encoded data

6 files changed, 133 insertions, 55 deletions

diff --git a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
index b8acc5a9..38cd18b7 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
@@ -28,14 +28,19 @@ import java.time.Duration
 import java.time.Instant
 import java.time.temporal.ChronoUnit
 import java.util.*
+import kotlinx.serialization.KSerializer
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
 
 /**
  * 32-byte Crockford's Base32 encoded data.
  */
-@Serializable()
-@JvmInline
-value class Base32Crockford32B(val encoded: String) {
-    init {
+@Serializable(with = Base32Crockford32B.Serializer::class)
+class Base32Crockford32B {
+    private var encoded: String? = null
+    val raw: ByteArray
+
+    constructor(encoded: String) {
         val decoded = try {
             Base32Crockford.decode(encoded) 
         } catch (e: EncodingException) {
@@ -48,16 +53,45 @@ value class Base32Crockford32B(val encoded: String) {
         require(decoded.size == 32) {
             "Encoded data should be 32 bytes long"
         }
+        this.raw = decoded
+        this.encoded = encoded
+    }
+    constructor(raw: ByteArray) {
+        require(raw.size == 32) {
+            "Encoded data should be 32 bytes long"
+        }
+        this.raw = raw
+    }
+
+    fun encoded(): String {
+        encoded = encoded ?: Base32Crockford.encode(raw)
+        return encoded!!
+    }
+
+    override fun equals(other: Any?) = (other is Base32Crockford32B) && Arrays.equals(raw, other.raw)
+
+    internal object Serializer : KSerializer<Base32Crockford32B> {
+        override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("Base32Crockford32B", PrimitiveKind.STRING)
+    
+        override fun serialize(encoder: Encoder, value: Base32Crockford32B) {
+            encoder.encodeString(value.encoded())
+        }
+    
+        override fun deserialize(decoder: Decoder): Base32Crockford32B {
+            return Base32Crockford32B(decoder.decodeString())
+        }
     }
 }
 
 /**
  * 64-byte Crockford's Base32 encoded data.
  */
-@Serializable()
-@JvmInline
-value class Base32Crockford64B(val encoded: String) {
-    init {
+@Serializable(with = Base32Crockford64B.Serializer::class)
+class Base32Crockford64B {
+    private var encoded: String? = null
+    val raw: ByteArray
+
+    constructor(encoded: String) {
         val decoded = try {
             Base32Crockford.decode(encoded) 
         } catch (e: EncodingException) {
@@ -68,7 +102,34 @@ value class Base32Crockford64B(val encoded: String) {
             "Data should be encoded using Crockford's Base32"
         }
         require(decoded.size == 64) {
-            "Encoded data should be 64 bytes long"
+            "Encoded data should be 32 bytes long"
+        }
+        this.raw = decoded
+        this.encoded = encoded
+    }
+    constructor(raw: ByteArray) {
+        require(raw.size == 64) {
+            "Encoded data should be 32 bytes long"
+        }
+        this.raw = raw
+    }
+
+    fun encoded(): String {
+        encoded = encoded ?: Base32Crockford.encode(raw)
+        return encoded!!
+    }
+
+    override fun equals(other: Any?) = (other is Base32Crockford64B) && Arrays.equals(raw, other.raw)
+
+    internal object Serializer : KSerializer<Base32Crockford64B> {
+        override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("Base32Crockford64B", PrimitiveKind.STRING)
+    
+        override fun serialize(encoder: Encoder, value: Base32Crockford64B) {
+            encoder.encodeString(value.encoded())
+        }
+    
+        override fun deserialize(decoder: Decoder): Base32Crockford64B {
+            return Base32Crockford64B(decoder.decodeString())
         }
     }
 }
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
index 8b90d499..cd16de3a 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
@@ -757,7 +757,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                                         (reserve_pub, bank_transaction) 
                                     VALUES (?, ?)
                                 """)
-                                stmt.setString(1, reservePub.encoded)
+                                stmt.setBytes(1, reservePub.raw)
                                 stmt.setLong(2, rowId)
                                 stmt.executeUpdate()
                                 conn.execSQLUpdate("NOTIFY incoming_tx, '${"${tx.creditorAccountId} $rowId"}'")
@@ -779,7 +779,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                                         (wtid, exchange_base_url, bank_transaction) 
                                     VALUES (?, ?, ?)
                                 """)
-                                stmt.setString(1, metadata.first.encoded)
+                                stmt.setBytes(1, metadata.first.raw)
                                 stmt.setString(2, metadata.second)
                                 stmt.setLong(3, rowId)
                                 stmt.executeUpdate()
@@ -963,7 +963,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                     getCurrency()
                 ),
                 debit_account = it.getString("debtor_payto_uri"),
-                reserve_pub = EddsaPublicKey(it.getString("reserve_pub")),
+                reserve_pub = EddsaPublicKey(it.getBytes("reserve_pub")),
             )
         }
     }
@@ -996,7 +996,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                     getCurrency()
                 ),
                 credit_account = it.getString("creditor_payto_uri"),
-                wtid = ShortHashCode(it.getString("wtid")),
+                wtid = ShortHashCode(it.getBytes("wtid")),
                 exchange_base_url = it.getString("exchange_base_url")
             )
         }
@@ -1399,16 +1399,16 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
     data class TalerTransferFromDb(
         val timestamp: Long,
         val debitTxRowId: Long,
-        val requestUid: String,
+        val requestUid: HashCode,
         val amount: TalerAmount,
         val exchangeBaseUrl: String,
-        val wtid: String,
+        val wtid: ShortHashCode,
         val creditAccount: String
     )
     /**
      * Gets a Taler transfer request, given its UID.
      */
-    fun talerTransferGetFromUid(requestUid: String): TalerTransferFromDb? = conn { conn ->
+    fun talerTransferGetFromUid(requestUid: HashCode): TalerTransferFromDb? = conn { conn ->
         val stmt = conn.prepareStatement("""
             SELECT
               wtid
@@ -1423,10 +1423,10 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                   ON bank_transaction=txs.bank_transaction_id
               WHERE request_uid = ?;
         """)
-        stmt.setString(1, requestUid)
+        stmt.setBytes(1, requestUid.raw)
         stmt.oneOrNull {
             TalerTransferFromDb(
-                wtid = it.getString("wtid"),
+                wtid = ShortHashCode(it.getBytes("wtid")),
                 amount = TalerAmount(
                     value = it.getLong("amount_value"),
                     frac = it.getInt("amount_frac"),
@@ -1473,6 +1473,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
         pmtInfId: String = "not used",
         endToEndId: String = "not used",
         ): TalerTransferCreationResult = conn { conn ->
+        val subject = "${req.wtid.encoded()} ${req.exchange_base_url}"
         val stmt = conn.prepareStatement("""
             SELECT
               out_exchange_balance_insufficient
@@ -1482,6 +1483,7 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                 taler_transfer (
                   ?,
                   ?,
+                  ?,
                   (?,?)::taler_amount,
                   ?,
                   ?,
@@ -1493,17 +1495,18 @@ class Database(dbConfig: String, private val bankCurrency: String): java.io.Clos
                 );
         """)
 
-        stmt.setString(1, req.request_uid.encoded)
-        stmt.setString(2, req.wtid.encoded)
-        stmt.setLong(3, req.amount.value)
-        stmt.setInt(4, req.amount.frac)
-        stmt.setString(5, req.exchange_base_url)
-        stmt.setString(6, stripIbanPayto(req.credit_account) ?: throw badRequest("credit_account payto URI is invalid"))
-        stmt.setLong(7, exchangeBankAccountId)
-        stmt.setLong(8, timestamp.toDbMicros() ?: throw faultyTimestampByBank())
-        stmt.setString(9, acctSvcrRef)
-        stmt.setString(10, pmtInfId)
-        stmt.setString(11, endToEndId)
+        stmt.setBytes(1, req.request_uid.raw)
+        stmt.setBytes(2, req.wtid.raw)
+        stmt.setString(3, subject)
+        stmt.setLong(4, req.amount.value)
+        stmt.setInt(5, req.amount.frac)
+        stmt.setString(6, req.exchange_base_url)
+        stmt.setString(7, stripIbanPayto(req.credit_account) ?: throw badRequest("credit_account payto URI is invalid"))
+        stmt.setLong(8, exchangeBankAccountId)
+        stmt.setLong(9, timestamp.toDbMicros() ?: throw faultyTimestampByBank())
+        stmt.setString(10, acctSvcrRef)
+        stmt.setString(11, pmtInfId)
+        stmt.setString(12, endToEndId)
 
         stmt.executeQuery().use {
             when {
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/WireGatewayApiHandlers.kt b/bank/src/main/kotlin/tech/libeufin/bank/WireGatewayApiHandlers.kt
index a7076e5e..8610a230 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/WireGatewayApiHandlers.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/WireGatewayApiHandlers.kt
@@ -66,14 +66,14 @@ fun Routing.talerWireGatewayHandlers(db: Database, ctx: BankApplicationContext)
         call.authCheck(TokenScope.readwrite, true)
         val req = call.receive<TransferRequest>()
         // Checking for idempotency.
-        val maybeDoneAlready = db.talerTransferGetFromUid(req.request_uid.encoded)
+        val maybeDoneAlready = db.talerTransferGetFromUid(req.request_uid)
         val creditAccount = stripIbanPayto(req.credit_account)
         if (maybeDoneAlready != null) {
             val isIdempotent =
                 maybeDoneAlready.amount == req.amount
                         && maybeDoneAlready.creditAccount == creditAccount
                         && maybeDoneAlready.exchangeBaseUrl == req.exchange_base_url
-                        && maybeDoneAlready.wtid == req.wtid.encoded
+                        && maybeDoneAlready.wtid == req.wtid
             if (isIdempotent) {
                 call.respond(
                     TransferResponse(
@@ -158,7 +158,7 @@ fun Routing.talerWireGatewayHandlers(db: Database, ctx: BankApplicationContext)
             )
         
         // TODO check conflict in transaction
-        if (db.bankTransactionCheckExists(req.reserve_pub.encoded) != null)
+        if (db.bankTransactionCheckExists(req.reserve_pub.encoded()) != null)
             throw conflict(
                 "Reserve pub. already used",
                 TalerErrorCode.TALER_EC_BANK_DUPLICATE_RESERVE_PUB_SUBJECT
@@ -176,7 +176,7 @@ fun Routing.talerWireGatewayHandlers(db: Database, ctx: BankApplicationContext)
             amount = req.amount,
             creditorAccountId = exchangeAccount.expectRowId(),
             transactionDate = txTimestamp,
-            subject = req.reserve_pub.encoded
+            subject = req.reserve_pub.encoded()
         )
         val res = db.bankTransactionCreate(op)
         /**
@@ -188,7 +188,7 @@ fun Routing.talerWireGatewayHandlers(db: Database, ctx: BankApplicationContext)
                 "Insufficient balance",
                 TalerErrorCode.TALER_EC_BANK_UNALLOWED_DEBIT
             )
-        val rowId = db.bankTransactionCheckExists(req.reserve_pub.encoded)
+        val rowId = db.bankTransactionCheckExists(req.reserve_pub.encoded())
             ?: throw internalServerError("Could not find the just inserted bank transaction")
         call.respond(
             AddIncomingResponse(
diff --git a/bank/src/test/kotlin/TalerApiTest.kt b/bank/src/test/kotlin/TalerApiTest.kt
index 728a7f28..c2abc619 100644
--- a/bank/src/test/kotlin/TalerApiTest.kt
+++ b/bank/src/test/kotlin/TalerApiTest.kt
@@ -53,11 +53,11 @@ class TalerApiTest {
         cashoutCurrency = "KUDOS"
     )
 
-    suspend fun transfer(db: Database, from: Long, to: BankAccount) {
-        db.talerTransferCreate(
+    suspend fun Database.genTransfer(from: Long, to: BankAccount) {
+        talerTransferCreate(
             req = TransferRequest(
                 request_uid = randHashCode(),
-                amount = TalerAmount(10, 0, "Kudos"),
+                amount = TalerAmount(10, 0, "KUDOS"),
                 exchange_base_url = "http://exchange.example.com/",
                 wtid = randShortHashCode(),
                 credit_account ="${stripIbanPayto(to.internalPaytoUri)}"
@@ -67,6 +67,21 @@ class TalerApiTest {
         )
     }
 
+    suspend fun Database.genIncoming(from: Long, to: Long) {
+        bankTransactionCreate(
+            BankInternalTransaction(
+                creditorAccountId = from,
+                debtorAccountId = to,
+                subject = randShortHashCode().encoded(),
+                amount = TalerAmount( 10, 0, "KUDOS"),
+                accountServicerReference = "acct-svcr-ref",
+                endToEndId = "end-to-end-id",
+                paymentInformationId = "pmtinfid",
+                transactionDate = Instant.now()
+            )
+        ).assertSuccess()
+    }
+
     fun commonSetup(lambda: (Database, BankApplicationContext) -> Unit) {
         setup { db, ctx -> 
             // Creating the exchange and merchant accounts first.
@@ -258,7 +273,7 @@ class TalerApiTest {
 
             // Foo pays Bar (the exchange) three time
             repeat(3) {
-                db.bankTransactionCreate(genTx(randShortHashCode().encoded)).assertSuccess()
+                db.genIncoming(2, 1)
             }
             // Should not show up in the taler wire gateway API history
             db.bankTransactionCreate(genTx("bogus foobar")).assertSuccess()
@@ -266,7 +281,7 @@ class TalerApiTest {
             db.bankTransactionCreate(genTx("payout", creditorId = 1, debtorId = 2)).assertSuccess()
             // Foo pays Bar (the exchange) twice, we should see five valid transactions
             repeat(2) {
-                db.bankTransactionCreate(genTx(randShortHashCode().encoded)).assertSuccess()
+                db.genIncoming(2, 1)
             }
 
             // Check ignore bogus subject
@@ -323,14 +338,14 @@ class TalerApiTest {
                     },
                     launch {
                         delay(200)
-                        db.bankTransactionCreate(genTx(randShortHashCode().encoded)).assertSuccess()
+                        db.genIncoming(2, 1)
                     }
                 )
             }
 
             // Testing ranges. 
             repeat(300) {
-                db.bankTransactionCreate(genTx(randShortHashCode().encoded)).assertSuccess()
+                db.genIncoming(2, 1)
             }
 
             // forward range:
@@ -397,7 +412,7 @@ class TalerApiTest {
 
             // Bar pays Foo three time
             repeat(3) {
-                transfer(db, 2, bankAccountFoo)
+                db.genTransfer(2, bankAccountFoo)
             }
             // Should not show up in the taler wire gateway API history
             db.bankTransactionCreate(genTx("bogus foobar", 1, 2)).assertSuccess()
@@ -405,7 +420,7 @@ class TalerApiTest {
             db.bankTransactionCreate(genTx("payout")).assertSuccess()
             // Bar pays Foo twice, we should see five valid transactions
             repeat(2) {
-                transfer(db, 2, bankAccountFoo)
+                db.genTransfer(2, bankAccountFoo)
             }
 
             // Check ignore bogus subject
@@ -462,14 +477,14 @@ class TalerApiTest {
                     },
                     launch {
                         delay(200)
-                        transfer(db, 2, bankAccountFoo)
+                        db.genTransfer(2, bankAccountFoo)
                     }
                 )
             }
 
             // Testing ranges.
             repeat(300) {
-                transfer(db, 2, bankAccountFoo)
+                db.genTransfer(2, bankAccountFoo)
             }
 
             // forward range:
diff --git a/database-versioning/libeufin-bank-0001.sql b/database-versioning/libeufin-bank-0001.sql
index aeeea0f7..85620117 100644
--- a/database-versioning/libeufin-bank-0001.sql
+++ b/database-versioning/libeufin-bank-0001.sql
@@ -360,8 +360,8 @@ CREATE TABLE IF NOT EXISTS bank_account_statements
 -- start of: Taler integration
 CREATE TABLE IF NOT EXISTS taler_exchange_outgoing
   (exchange_outgoing_id BIGINT GENERATED BY DEFAULT AS IDENTITY
-  ,request_uid TEXT UNIQUE DEFAULT NULL
-  ,wtid TEXT NOT NULL UNIQUE
+  ,request_uid BYTEA UNIQUE CHECK (LENGTH(request_uid)=64)
+  ,wtid BYTEA NOT NULL UNIQUE CHECK (LENGTH(wtid)=32)
   ,exchange_base_url TEXT NOT NULL
   ,bank_transaction BIGINT UNIQUE NOT NULL
     REFERENCES bank_account_transactions(bank_transaction_id)
@@ -371,7 +371,7 @@ CREATE TABLE IF NOT EXISTS taler_exchange_outgoing
 
 CREATE TABLE IF NOT EXISTS taler_exchange_incoming
   (exchange_incoming_id BIGINT GENERATED BY DEFAULT AS IDENTITY
-  ,reserve_pub TEXT NOT NULL UNIQUE
+  ,reserve_pub BYTEA NOT NULL UNIQUE CHECK (LENGTH(reserve_pub)=32)
   ,bank_transaction BIGINT UNIQUE NOT NULL
     REFERENCES bank_account_transactions(bank_transaction_id)
       ON DELETE RESTRICT
diff --git a/database-versioning/procedures.sql b/database-versioning/procedures.sql
index f1fa0b9c..c3cab648 100644
--- a/database-versioning/procedures.sql
+++ b/database-versioning/procedures.sql
@@ -196,8 +196,9 @@ COMMENT ON FUNCTION customer_delete(TEXT)
   IS 'Deletes a customer (and its bank account via cascade) if the balance is zero';
 
 CREATE OR REPLACE FUNCTION taler_transfer(
-  IN in_request_uid TEXT,
-  IN in_wtid TEXT,
+  IN in_request_uid BYTEA,
+  IN in_wtid BYTEA,
+  IN in_subject TEXT,
   IN in_amount taler_amount,
   IN in_exchange_base_url TEXT,
   IN in_credit_account_payto TEXT,
@@ -214,7 +215,6 @@ LANGUAGE plpgsql
 AS $$
 DECLARE
 receiver_bank_account_id BIGINT;
-payment_subject TEXT;
 BEGIN
 
 -- First creating the bank transaction, then updating
@@ -232,8 +232,6 @@ THEN
   RETURN;
 END IF;
 out_nx_creditor=FALSE;
-SELECT CONCAT(in_wtid, ' ', in_exchange_base_url)
-  INTO payment_subject;
 SELECT
   out_balance_insufficient,
   out_debit_row_id
@@ -243,7 +241,7 @@ SELECT
   FROM bank_wire_transfer(
     receiver_bank_account_id,
     in_exchange_bank_account_id,
-    payment_subject,
+    in_subject,
     in_amount,
     in_timestamp,
     in_account_servicer_reference,
@@ -269,7 +267,8 @@ INSERT
 PERFORM pg_notify('outgoing_tx', in_exchange_bank_account_id || ' ' || out_tx_row_id);
 END $$;
 COMMENT ON FUNCTION taler_transfer(
-  text,
+  bytea,
+  bytea,
   text,
   taler_amount,
   text,