Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,34 @@ interface AssetLockDao {
@Query("SELECT * FROM asset_locks WHERE outPointHex = :outPointHex")
suspend fun getByOutPointHex(outPointHex: String): AssetLockEntity?

/**
* Transaction-label resolver probe: the `fundingTypeRaw` of the asset
* lock whose outpoint belongs to [txidHex]. [txidHex] is the explorer
* DISPLAY txid hex (64 chars, wire order reversed; uppercase input is
* canonicalized via `lower()`) — the prefix of the `outPointHex` PK
* (`<txidDisplayHex>:<vout>`). The query enforces the 64-hex input
* contract itself (length + hex-only GLOB, matching the
* null-on-malformed behavior of
* [TransactionDao.transactionKindForDisplayTxid]) and compares the
* exact 65-char `<txid>:` prefix rather than a LIKE pattern, so
* `%`/`_` in malformed input can never match arbitrary rows. Returns
* null for malformed input or when no asset lock funds this tx (e.g.
* plain send, or an AssetUnlock/unshield — see
* [TransactionDao.transactionKindForTxid]).
*
* DIP-0027 permits several asset-lock outputs in one transaction;
* those rows are created by the same funding flow and share a
* `fundingTypeRaw`, so the unordered `LIMIT 1` pick is value-stable.
*/
@Query(
"SELECT fundingTypeRaw FROM asset_locks " +
"WHERE length(:txidHex) = 64 " +
"AND lower(:txidHex) NOT GLOB '*[^0-9a-f]*' " +
"AND substr(outPointHex, 1, 65) = lower(:txidHex) || ':' " +
"LIMIT 1"
)
suspend fun fundingTypeForTxid(txidHex: String): Int?
Comment on lines +96 to +103

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Validate txids before using them as a LIKE pattern

Unlike transactionKindForDisplayTxid, this resolver does not enforce its documented 64-character hexadecimal input contract. SQLite interprets % and _ in the bound value as wildcards, so malformed inputs such as % or 64 underscores match unrelated asset-lock rows and LIMIT 1 returns one row's funding type rather than null. Validate and canonicalize the input in SQL, then compare the exact 64-character outpoint prefix; generated outPointHex values are lowercase, while lower(:txidHex) preserves support for uppercase canonical txids.

Suggested change
@Query(
"SELECT fundingTypeRaw FROM asset_locks " +
"WHERE outPointHex LIKE :txidHex || ':%' LIMIT 1"
)
suspend fun fundingTypeForTxid(txidHex: String): Int?
@Query(
"SELECT fundingTypeRaw FROM asset_locks " +
"WHERE length(:txidHex) = 64 " +
"AND lower(:txidHex) NOT GLOB '*[^0-9a-f]*' " +
"AND substr(outPointHex, 1, 65) = lower(:txidHex) || ':' " +
"LIMIT 1"
)
suspend fun fundingTypeForTxid(txidHex: String): Int?

source: ['codex']

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — Validate txids before using them as a LIKE pattern no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

Comment on lines +96 to +103

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: fundingTypeForTxid doesn't enforce its own hex-only contract before using input as a LIKE pattern

fundingTypeForTxid binds txidHex directly into outPointHex LIKE :txidHex || ':%' without validating it is a 64-char pure-hex string, unlike the sibling transactionKindForDisplayTxid which enforces exactly that via displayHexToWireTxid and returns null on malformed input. SQLite treats %/_ in the bound value as wildcards, so fundingTypeForTxid("%") matches an arbitrary asset-lock row and 64 _ chars matches every row — with LIMIT 1 this silently returns a wrong funding type instead of null. The in-code comment's claim that "a txid is a pure-hex string ... so the LIKE pattern carries no wildcards of its own" is an assumption about callers, not an invariant this public method enforces. Separately, even once the pattern is exact-matched, LIMIT 1 with no ORDER BY over a genuinely one-to-many relationship (DIP-0027 permits multiple asset-lock outputs sharing a txid) makes the returned row scan-order dependent; if all vouts for a txid always share the same funding type this is harmless, but that invariant should be documented or the query made deterministic.

Suggested change
@Query(
"SELECT fundingTypeRaw FROM asset_locks " +
"WHERE outPointHex LIKE :txidHex || ':%' LIMIT 1"
)
suspend fun fundingTypeForTxid(txidHex: String): Int?
@Query(
"SELECT fundingTypeRaw FROM asset_locks " +
"WHERE length(:txidHex) = 64 " +
"AND lower(:txidHex) NOT GLOB '*[^0-9a-f]*' " +
"AND substr(outPointHex, 1, 65) = lower(:txidHex) || ':' " +
"LIMIT 1"
)
suspend fun fundingTypeForTxid(txidHex: String): Int?

source: ['claude', 'codex']

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 176f8ed — adopted the suggested query shape: the SQL now enforces the 64-hex contract itself (length(:txidHex) = 64 + hex-only GLOB, lower()-canonicalized) and compares the exact 65-char <txid>: prefix via substr, so there is no LIKE pattern left for %/_ to exploit — fundingTypeForTxid("%") and 64 underscores now return null instead of an arbitrary row's funding type. The multi-vout LIMIT 1 point is addressed in the KDoc: DIP-0027 rows for one txid come from the same funding flow and share fundingTypeRaw, so the unordered pick is value-stable (and the new test pins that with two vouts sharing a txid).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 176f8edfundingTypeForTxid doesn't enforce its own hex-only contract before using input as a LIKE pattern no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.


@Upsert
suspend fun upsert(assetLock: AssetLockEntity)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,35 @@ interface TransactionDao {
@Query("SELECT * FROM transactions WHERE txid = :txid")
suspend fun getByTxid(txid: ByteArray): TransactionEntity?

/**
* Transaction-label resolver probe for the withdraw/unshield case,
* which has NO `asset_locks` row and is identified solely by
* `transactionTypeKind == 7` (AssetUnlock).
*
* [txidWire] is the raw little-endian WIRE txid — the exact
* [TransactionEntity.txid] PK form (a BLOB), i.e. the explorer display
* hex reversed then hex-decoded. When the caller holds the display hex
* (as the resolver does), prefer [transactionKindForDisplayTxid], which
* does the reversal. Returns null when no such tx is stored;
* `transactionTypeKind == 0xFF` (255) means "not yet populated".
*/
@Query("SELECT transactionTypeKind FROM transactions WHERE txid = :txidWire LIMIT 1")
suspend fun transactionKindForTxid(txidWire: ByteArray): Int?

/**
* Convenience over [transactionKindForTxid] keyed by the explorer
* DISPLAY txid hex (64 lowercase chars, wire order reversed) the
* resolver already holds — the same display form used by
* [AssetLockDao.fundingTypeForTxid]. Reverses to wire order before the
* BLOB PK match. Returns null for malformed hex (not 64 hex chars) or
* when no such tx is stored. Not a Room query — a plain default method
* delegating to [transactionKindForTxid].
*/
suspend fun transactionKindForDisplayTxid(txidDisplayHex: String): Int? {
val wire = displayHexToWireTxid(txidDisplayHex) ?: return null
return transactionKindForTxid(wire)
Comment on lines +42 to +56

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Add Room tests for the new resolver contracts

No SDK test invokes transactionKindForTxid, transactionKindForDisplayTxid, or fundingTypeForTxid. Add in-memory Room coverage using a non-palindromic txid to pin the display-to-wire byte reversal, plus missing, malformed, and uppercase input cases. The asset-lock tests should also cover multiple vouts sharing a txid and define the intended behavior if their funding types differ, because the current nullable scalar query uses LIMIT 1 and otherwise leaves that selection dependent on row scan order.

source: ['codex']

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — Add Room tests for the new resolver contracts no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

Comment on lines +42 to +56

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: No test coverage for the three new resolver methods

Nothing in the repo invokes transactionKindForTxid, transactionKindForDisplayTxid, or fundingTypeForTxid. The hand-written byte-reversal in displayHexToWireTxid (TransactionDao.kt:129-141) has no pinned regression test — deleting the reversal loop entirely would not fail CI, silently mislabeling every withdraw/unshield transaction on-device. The in-memory Room harness already used by WalletDeletionTest is available for this. Add cases with a non-palindromic txid (to catch endian regressions a symmetric fixture like "ab".repeat(32) would miss), uppercase input, malformed/short hex, no-match, and — for fundingTypeForTxid — multiple asset-lock rows sharing a txid prefix to pin down the LIMIT 1 selection behavior.

source: ['claude', 'codex']

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 176f8ed: TransactionLabelResolverDaoTest (in-memory Room via the same Robolectric harness as WalletDeletionTest) covers all three resolvers with a NON-palindromic txid (0x01..0x20), pinning the display→wire byte reversal — the display hex must resolve while the unreversed wire hex must miss, so deleting the reversal loop in displayHexToWireTxid now fails CI. Also covered: uppercase input, malformed/short/empty hex, no-match, wildcard inputs (%, 64 underscores) against fundingTypeForTxid, and multiple asset-lock vouts sharing a txid to pin the LIMIT 1 value-stability.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — No test coverage for the three new resolver methods no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

}

/** Timeline join helper — parents of a wallet's TXO set. */
@Query("SELECT * FROM transactions WHERE txid IN (:txids) ORDER BY firstSeen DESC")
fun observeByTxids(txids: List<ByteArray>): Flow<List<TransactionEntity>>
Expand Down Expand Up @@ -89,4 +118,26 @@ interface TransactionDao {
/** StorageExplorer row count. */
@Query("SELECT COUNT(*) FROM transactions")
fun count(): Flow<Long>

companion object {
/**
* Explorer DISPLAY txid hex (wire order reversed, 64 lowercase
* chars) → raw little-endian WIRE txid, the [TransactionEntity.txid]
* PK form. Mirrors the txid half of [decodeOutPointHex]. Returns
* null for any non-64-char or non-hex input.
*/
private fun displayHexToWireTxid(displayHex: String): ByteArray? {
if (displayHex.length != 64) return null
val display = ByteArray(32)
for (i in 0 until 32) {
val hi = Character.digit(displayHex[i * 2], 16)
val lo = Character.digit(displayHex[i * 2 + 1], 16)
if (hi < 0 || lo < 0) return null
display[i] = ((hi shl 4) or lo).toByte()
}
val wire = ByteArray(32)
for (i in 0 until 32) wire[i] = display[31 - i]
return wire
}
Comment on lines +129 to +141

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: displayHexToWireTxid accepts non-ASCII Unicode hex digits via Character.digit

Java's Character.digit(char, 16) is documented to accept Unicode decimal digits (any script where isDigit() is true and the decimal value fits the radix, e.g. Arabic-Indic ١) and fullwidth Latin letters (-, -) in addition to ASCII 0-9a-fA-F. A 64-character string built from these non-ASCII equivalents therefore decodes successfully instead of returning null, contradicting the method's documented contract ("Returns null for malformed hex (not 64 hex chars)") and diverging from AssetLockDao.fundingTypeForTxid's new ASCII-only NOT GLOB '*[^0-9a-f]*' guard. Practical exposure is limited today since no caller yet exists in the tree, but this is new SDK-public surface. Note this exact Character.digit pattern is intentionally mirrored from the pre-existing decodeOutPointHex in PlatformWalletPersistenceHandler.kt (which the doc comment explicitly cites), so patching only this copy would leave an inconsistent sibling; the two hex-parsing loops (already flagged separately as duplicated) should be consolidated into one ASCII-only helper shared by both.

Suggested change
private fun displayHexToWireTxid(displayHex: String): ByteArray? {
if (displayHex.length != 64) return null
val display = ByteArray(32)
for (i in 0 until 32) {
val hi = Character.digit(displayHex[i * 2], 16)
val lo = Character.digit(displayHex[i * 2 + 1], 16)
if (hi < 0 || lo < 0) return null
display[i] = ((hi shl 4) or lo).toByte()
}
val wire = ByteArray(32)
for (i in 0 until 32) wire[i] = display[31 - i]
return wire
}
private fun displayHexToWireTxid(displayHex: String): ByteArray? {
if (displayHex.length != 64) return null
val display = ByteArray(32)
for (i in 0 until 32) {
val hi = asciiHexDigit(displayHex[i * 2]) ?: return null
val lo = asciiHexDigit(displayHex[i * 2 + 1]) ?: return null
display[i] = ((hi shl 4) or lo).toByte()
}
val wire = ByteArray(32)
for (i in 0 until 32) wire[i] = display[31 - i]
return wire
}
private fun asciiHexDigit(char: Char): Int? = when (char) {
in '0'..'9' -> char - '0'
in 'a'..'f' -> char - 'a' + 10
in 'A'..'F' -> char - 'A' + 10
else -> null
}

source: ['claude', 'codex']

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package org.dashfoundation.dashsdk.persistence

import androidx.test.core.app.ApplicationProvider
import kotlinx.coroutines.test.runTest
import org.dashfoundation.dashsdk.persistence.entities.AssetLockEntity
import org.dashfoundation.dashsdk.persistence.entities.TransactionEntity
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner

/**
* In-memory Room contract tests for the three transaction-label resolver
* queries: [org.dashfoundation.dashsdk.persistence.dao.TransactionDao]'s
* `transactionKindForTxid` / `transactionKindForDisplayTxid` and
* [org.dashfoundation.dashsdk.persistence.dao.AssetLockDao]'s
* `fundingTypeForTxid`.
*
* The fixture txid is deliberately NON-palindromic (`0x01..0x20`), so the
* display↔wire byte reversal in `displayHexToWireTxid` is pinned: deleting
* (or double-applying) the reversal loop flips these assertions, which a
* symmetric fixture like `"ab" * 32` would not catch.
*/
@RunWith(RobolectricTestRunner::class)
class TransactionLabelResolverDaoTest {

private lateinit var db: DashDatabase

private val walletId = ByteArray(32) { 1 }

/** Raw little-endian WIRE txid — the `transactions.txid` PK form. */
private val wireTxid = ByteArray(32) { (it + 1).toByte() }

/** Explorer DISPLAY hex: wire bytes reversed, lowercase. */
private val displayHex = hex(wireTxid.reversedArray())

/** The unreversed hex of the wire bytes — display form of a DIFFERENT tx. */
private val wireHexUnreversed = hex(wireTxid)

/** `TransactionType` discriminant for AssetUnlock (withdraw/unshield). */
private val kindAssetUnlock = 7

/** `AssetLockFundingType` discriminant for AssetLockAddressTopUp. */
private val fundingTypeAddressTopUp = 4

@Before
fun setUp() {
db = DashDatabase.createInMemory(ApplicationProvider.getApplicationContext())
}

@After
fun tearDown() {
db.close()
}

private fun hex(bytes: ByteArray): String =
bytes.joinToString("") { "%02x".format(it) }

private suspend fun insertTransaction() {
db.transactionDao().upsert(
TransactionEntity(
txid = wireTxid,
transactionData = ByteArray(4),
transactionTypeKind = kindAssetUnlock,
),
)
}

private suspend fun insertAssetLock(outPointHex: String, fundingTypeRaw: Int) {
db.assetLockDao().upsert(
AssetLockEntity(
outPointHex = outPointHex,
walletId = walletId,
transactionBytes = ByteArray(4),
fundingTypeRaw = fundingTypeRaw,
identityIndexRaw = 0,
amountDuffs = 10_000,
statusRaw = 1,
),
)
}

// ── transactionKindForTxid (wire BLOB PK) ─────────────────────────

@Test
fun transactionKindForTxidMatchesWireBytes() = runTest {
insertTransaction()
assertEquals(kindAssetUnlock, db.transactionDao().transactionKindForTxid(wireTxid))
}

@Test
fun transactionKindForTxidReturnsNullForUnknownTxid() = runTest {
insertTransaction()
assertNull(db.transactionDao().transactionKindForTxid(ByteArray(32) { 9 }))
}

// ── transactionKindForDisplayTxid (display hex → wire reversal) ───

@Test
fun displayTxidLookupReversesToWireOrder() = runTest {
insertTransaction()
// Display hex resolves the row — pins the byte reversal on a
// non-palindromic txid.
assertEquals(
kindAssetUnlock,
db.transactionDao().transactionKindForDisplayTxid(displayHex),
)
// The UNREVERSED wire hex must miss: it denotes a different
// (reversed) txid. If the reversal loop were deleted, this lookup
// would incorrectly succeed and the one above would fail.
assertNull(db.transactionDao().transactionKindForDisplayTxid(wireHexUnreversed))
}

@Test
fun displayTxidLookupAcceptsUppercaseHex() = runTest {
insertTransaction()
assertEquals(
kindAssetUnlock,
db.transactionDao().transactionKindForDisplayTxid(displayHex.uppercase()),
)
}

@Test
fun displayTxidLookupRejectsMalformedInput() = runTest {
insertTransaction()
assertNull(db.transactionDao().transactionKindForDisplayTxid(""))
assertNull(db.transactionDao().transactionKindForDisplayTxid(displayHex.dropLast(1)))
assertNull(db.transactionDao().transactionKindForDisplayTxid(displayHex + "00"))
assertNull(
db.transactionDao()
.transactionKindForDisplayTxid("z" + displayHex.drop(1)),
)
}

// ── fundingTypeForTxid (outPointHex prefix) ───────────────────────

@Test
fun fundingTypeForTxidMatchesExactOutpointPrefix() = runTest {
insertAssetLock("$displayHex:0", fundingTypeAddressTopUp)
assertEquals(
fundingTypeAddressTopUp,
db.assetLockDao().fundingTypeForTxid(displayHex),
)
// Uppercase canonical txids are accepted (`lower()` in the query).
assertEquals(
fundingTypeAddressTopUp,
db.assetLockDao().fundingTypeForTxid(displayHex.uppercase()),
)
// A different txid misses even with rows present.
assertNull(db.assetLockDao().fundingTypeForTxid(wireHexUnreversed))
}

@Test
fun fundingTypeForTxidRejectsWildcardAndMalformedInput() = runTest {
insertAssetLock("$displayHex:0", fundingTypeAddressTopUp)
// SQLite LIKE would have treated these as wildcards; the exact
// substr comparison + hex GLOB guard must return null instead of
// an arbitrary row's funding type.
assertNull(db.assetLockDao().fundingTypeForTxid("%"))
assertNull(db.assetLockDao().fundingTypeForTxid("_".repeat(64)))
assertNull(db.assetLockDao().fundingTypeForTxid("%".repeat(64)))
assertNull(db.assetLockDao().fundingTypeForTxid(displayHex.dropLast(1)))
assertNull(db.assetLockDao().fundingTypeForTxid(""))
}

@Test
fun fundingTypeForTxidIsStableAcrossMultipleVoutsOfOneTx() = runTest {
// DIP-0027 permits several asset-lock outputs per tx; rows from the
// same funding flow share fundingTypeRaw, so LIMIT 1 stays
// value-stable regardless of which row the scan picks.
insertAssetLock("$displayHex:0", fundingTypeAddressTopUp)
insertAssetLock("$displayHex:1", fundingTypeAddressTopUp)
// An unrelated lock with a different funding type must not bleed in.
insertAssetLock("${hex(ByteArray(32) { 9 }.reversedArray())}:0", 5)
assertEquals(
fundingTypeAddressTopUp,
db.assetLockDao().fundingTypeForTxid(displayHex),
)
}
}
Loading