feat: add missing tests to btcindexer storage#354
Merged
Conversation
Contributor
Reviewer's GuideAdds comprehensive edge-case and behavior tests for the btcindexer Cloudflare storage layer, covering nBTC mint transaction lifecycle, block processing logic, and helper query functions, plus a minor type import addition. Sequence diagram for nbtc mint tx lifecycle edge cases tested in PRsequenceDiagram
actor IndexerWorker
participant CFStorage
Note over IndexerWorker,CFStorage: Broadcast registration without block info
IndexerWorker->>CFStorage: registerBroadcastedNbtcTx([NbtcBroadcastedDeposit])
CFStorage-->>IndexerWorker: ok (status Broadcasting)
Note over IndexerWorker,CFStorage: Later update with block info
IndexerWorker->>CFStorage: insertOrUpdateNbtcTxs([txBase])
CFStorage-->>IndexerWorker: ok (status Confirming)
Note over IndexerWorker,CFStorage: Query single tx status
IndexerWorker->>CFStorage: getTxStatus(txId)
CFStorage-->>IndexerWorker: MintTxStatus.Confirming or null
Note over IndexerWorker,CFStorage: Batch failure handling and retries
loop for each failure
IndexerWorker->>CFStorage: batchUpdateNbtcMintTxs([update MintFailed])
CFStorage-->>IndexerWorker: ok (increment retry_count)
end
Note over IndexerWorker,CFStorage: Finalization and candidate selection
IndexerWorker->>CFStorage: finalizeNbtcTxs([txId])
CFStorage-->>IndexerWorker: ok
IndexerWorker->>CFStorage: getNbtcMintCandidates(maxRetryCount)
CFStorage-->>IndexerWorker: [NbtcMintTx] within retry limit
Class diagram for CFStorage nbtc mint and block operations tested in PRclassDiagram
class CFStorage {
+insertOrUpdateNbtcTxs(nbtcTxs)
+getTxStatus(txId)
+updateNbtcTxsStatus(txIds, status)
+finalizeNbtcTxs(txIds)
+batchUpdateNbtcMintTxs(updates)
+getNbtcMintTx(txId)
+getNbtcMintTxsBySuiAddr(suiAddress)
+getNbtcMintTxsByBtcSender(btcSender, btcNetwork)
+getNbtcMintCandidates(maxRetryCount)
+registerBroadcastedNbtcTx(broadcastedDeposits)
+getConfirmingBlocks()
+getLatestBlockHeight(btcNetwork)
+getChainTip(btcNetwork)
+setChainTip(height, btcNetwork)
+getBlock(hash)
+getBlockHash(height, btcNetwork)
+insertBlockInfo(blockInfo)
+markBlockAsProcessed(hash, btcNetwork)
+getBlocksToProcess(limit)
}
class NbtcMintTx {
+tx_id
+vout
+status
+retry_count
+block_hash
+block_height
+sui_tx_digest
+sui_recipient
+btc_sender
+amount
}
class NbtcBroadcastedDeposit {
+txId
+btcNetwork
+suiNetwork
+nbtcPkg
+depositAddress
+sender
+vout
+suiRecipient
+amount
}
class BlockInfo {
+hash
+height
+network
+timestamp_ms
+processed
}
class MintTxStatus {
<<enumeration>>
Confirming
Broadcasting
Minted
MintFailed
}
class InsertBlockStatus {
<<enumeration>>
Inserted
Skipped
}
class BtcNet {
<<enumeration>>
REGTEST
TESTNET
MAINNET
}
class D1Database {
+prepare(sql)
+run()
+all()
}
class StorageHelpers {
+fetchPackageConfigs(db)
+fetchNbtcAddresses(db)
}
CFStorage "1" --> "*" NbtcMintTx : manages
CFStorage "1" --> "*" BlockInfo : manages
CFStorage ..> NbtcBroadcastedDeposit : registers
CFStorage ..> MintTxStatus : uses
CFStorage ..> InsertBlockStatus : returns
CFStorage ..> BtcNet : filters_by
StorageHelpers ..> D1Database : queries
StorageHelpers ..> NbtcMintTx : returns_related_data
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Some test names/descriptions don’t match their assertions (e.g.
fetchPackageConfigs should return empty array when no active packagesexpects length1, andfetchNbtcAddresses should return empty map when no active setupsexpectssize1), which makes the intended behavior unclear—either adjust the expectations or rename the tests to reflect what they actually verify. - A few of the new edge-case tests only assert
not.toBeNull()on results for empty inputs (e.g.insertOrUpdateNbtcTxs/updateNbtcTxsStatus/finalizeNbtcTxswith empty arrays); consider tightening these to assert on more concrete behavior (such as no-op semantics or specific return values) to better document and lock in the intended contract.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Some test names/descriptions don’t match their assertions (e.g. `fetchPackageConfigs should return empty array when no active packages` expects length `1`, and `fetchNbtcAddresses should return empty map when no active setups` expects `size` `1`), which makes the intended behavior unclear—either adjust the expectations or rename the tests to reflect what they actually verify.
- A few of the new edge-case tests only assert `not.toBeNull()` on results for empty inputs (e.g. `insertOrUpdateNbtcTxs` / `updateNbtcTxsStatus` / `finalizeNbtcTxs` with empty arrays); consider tightening these to assert on more concrete behavior (such as no-op semantics or specific return values) to better document and lock in the intended contract.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b6c8878 to
5ce1b19
Compare
sczembor
approved these changes
Feb 13, 2026
Signed-off-by: Ravindra Meena <rmeena840@gmail.com>
5ce1b19 to
a6a6ff9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes: #349
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdSummary by Sourcery
Expand test coverage for btcindexer storage edge cases and helper functions.
Tests: