feat: add missing tests to the sui-indexer storage#361
Open
feat: add missing tests to the sui-indexer storage#361
Conversation
Contributor
Reviewer's GuideAdds comprehensive unit tests around Sui indexer storage behavior for Sui GraphQL cursors and NBTC redeem flows, covering cursor persistence, redeem existence and retrieval, signing metadata, broadcasting, and finalization side-effects on UTXOs. ER diagram for Sui indexer state and NBTC redeem tables testederDiagram
INDEXER_STATE {
int setup_id PK
string nbtc_cursor
int updated_at
}
NBTC_REDEEM_REQUESTS {
int redeem_id PK
int setup_id FK
string redeemer
string recipient_script
int amount
int fee
string sui_tx
string status
string btc_tx
}
NBTC_UTXOS {
int nbtc_utxo_id PK
int setup_id FK
string script_pubkey
string dwallet_id
string txid
int vout
int amount
string status
}
REDEEM_INPUTS {
int redeem_id FK
int utxo_id FK
int input_index
string dwallet_id
int created_at
string sign_id
}
INDEXER_STATE ||--o{ NBTC_REDEEM_REQUESTS : has_redeem_requests
INDEXER_STATE ||--o{ NBTC_UTXOS : has_utxos
NBTC_REDEEM_REQUESTS ||--o{ REDEEM_INPUTS : has_inputs
NBTC_UTXOS ||--o{ REDEEM_INPUTS : referenced_by_inputs
Class diagram for IndexerStorage redeem and cursor operationsclassDiagram
class IndexerStorage {
getMultipleSuiGqlCursors(setupIds)
saveMultipleSuiGqlCursors(entries)
getSuiGqlCursor(setupId)
hasRedeemRequest(redeemId)
getRedeemUtxosWithDetails(redeemId)
getRedeemRequestData(redeemId)
getSignedRedeems()
getRedeemWithSetup(redeemId)
saveRedeemInputs(inputs)
getRedeemInputs(redeemId)
updateRedeemInputSig(redeemId, utxoId, signId)
clearRedeemInputSignId(redeemId, utxoId)
getRedeemInfoBySignId(signId)
markRedeemBroadcasted(redeemId, btcTxId)
setRedeemFinalized(redeemId)
}
class IndexerState {
int setup_id
string nbtc_cursor
int updated_at
}
class RedeemRequest {
int redeem_id
int setup_id
string redeemer
string recipient_script
int amount
int fee
string sui_tx
RedeemRequestStatus status
string btc_tx
}
class RedeemInput {
int redeem_id
int utxo_id
int input_index
string dwallet_id
int created_at
string sign_id
}
class NbtcUtxo {
int nbtc_utxo_id
int setup_id
string script_pubkey
string dwallet_id
string txid
int vout
int amount
UtxoStatus status
}
class RedeemRequestStatus {
<<enumeration>>
Pending
Signed
Broadcasting
Finalized
}
class UtxoStatus {
<<enumeration>>
Available
Spent
}
IndexerStorage --> IndexerState : uses
IndexerStorage --> RedeemRequest : manages
IndexerStorage --> RedeemInput : manages
IndexerStorage --> NbtcUtxo : manages
RedeemRequestStatus <.. RedeemRequest : status
UtxoStatus <.. NbtcUtxo : status
RedeemRequest "1" --> "*" RedeemInput : has
RedeemInput "*" --> "1" NbtcUtxo : references
IndexerState "1" --> "*" RedeemRequest : setup_for
IndexerState "1" --> "*" NbtcUtxo : setup_for
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
rmeena840
force-pushed
the
348-add-missing-tests-to-sui-indexer-storage
branch
2 times, most recently
from
a1bbafc to
ebe72cf
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds additional unit test coverage for D1Storage behaviors in the Sui indexer, focusing on Sui GraphQL cursor storage and redeem-related read/write helpers to ensure SQL queries are exercised.
Changes:
- Add tests for reading/saving Sui GraphQL cursors in
indexer_state. - Add tests for redeem-request existence checks and redeem data retrieval helpers.
- Add tests for redeem input sign-id clearing, sign-id lookup, broadcast marking, and finalization (including UTXO status updates).
Comment on lines
+608
to
+609
| const cursors = await storage.getMultipleSuiGqlCursors([1]); | ||
| expect(cursors[1]).toBe("cursor1"); |
There was a problem hiding this comment.
This test claims to cover multiple setups, but it only queries a single setupId. To validate the intended behavior of getMultipleSuiGqlCursors, pass at least two setupIds and assert that missing setups are present in the result with a null cursor (the implementation pre-seeds the map with nulls).
Suggested change
| const cursors = await storage.getMultipleSuiGqlCursors([1]); | |
| expect(cursors[1]).toBe("cursor1"); | |
| const cursors = await storage.getMultipleSuiGqlCursors([1, 2]); | |
| expect(cursors[1]).toBe("cursor1"); | |
| expect(cursors[2]).toBeNull(); |
Comment on lines
+612
to
+615
| it("saveMultipleSuiGqlCursors should save cursors", async () => { | ||
| await storage.saveMultipleSuiGqlCursors([{ setupId: 1, cursor: "newCursor" }]); | ||
| const cursor = await storage.getSuiGqlCursor(1); | ||
| expect(cursor).toBe("newCursor"); |
There was a problem hiding this comment.
The test name says "save ... cursors" but only exercises a single cursor insert. Consider saving multiple cursors and also covering the ON CONFLICT update path (e.g., save an initial cursor then save again for the same setupId) so this test actually verifies the multi-save/upsert behavior.
Suggested change
| it("saveMultipleSuiGqlCursors should save cursors", async () => { | |
| await storage.saveMultipleSuiGqlCursors([{ setupId: 1, cursor: "newCursor" }]); | |
| const cursor = await storage.getSuiGqlCursor(1); | |
| expect(cursor).toBe("newCursor"); | |
| it("saveMultipleSuiGqlCursors should save cursors and update existing ones", async () => { | |
| // Initial save with multiple cursors | |
| await storage.saveMultipleSuiGqlCursors([ | |
| { setupId: 1, cursor: "cursor1-initial" }, | |
| { setupId: 2, cursor: "cursor2-initial" }, | |
| ]); | |
| let cursors = await storage.getMultipleSuiGqlCursors([1, 2]); | |
| expect(cursors[1]).toBe("cursor1-initial"); | |
| expect(cursors[2]).toBe("cursor2-initial"); | |
| // Save again for an existing setupId to exercise ON CONFLICT/update, | |
| // and add a new setupId at the same time. | |
| await storage.saveMultipleSuiGqlCursors([ | |
| { setupId: 1, cursor: "cursor1-updated" }, | |
| { setupId: 3, cursor: "cursor3-initial" }, | |
| ]); | |
| cursors = await storage.getMultipleSuiGqlCursors([1, 2, 3]); | |
| expect(cursors[1]).toBe("cursor1-updated"); | |
| expect(cursors[2]).toBe("cursor2-initial"); | |
| expect(cursors[3]).toBe("cursor3-initial"); | |
| const singleCursor = await storage.getSuiGqlCursor(1); | |
| expect(singleCursor).toBe("cursor1-updated"); |
Contributor
Author
There was a problem hiding this comment.
it has been merged
| it("getRedeemRequestData should return recipient_script and amount", async () => { | ||
| await insertRedeemRequest(storage, 1, "redeemer1", recipientScript, 3000, 1000, "0xSuiTx1"); | ||
| const data = await storage.getRedeemRequestData(1); | ||
| expect(data).not.toBeNull(); |
There was a problem hiding this comment.
This test description says it should return both recipient_script and amount, but it only asserts amount. To fully verify the query/mapping, also assert that recipient_script matches the inserted recipientScript (byte-for-byte).
Suggested change
| expect(data).not.toBeNull(); | |
| expect(data).not.toBeNull(); | |
| expect(data!.recipient_script).toEqual(recipientScript); |
Contributor
robert-zaremba
left a comment
robert-zaremba
left a comment
There was a problem hiding this comment.
too much boilerplate / repeated code. Please simplify
| "INSERT INTO indexer_state (setup_id, nbtc_cursor, updated_at) VALUES (?, ?, ?)", | ||
| ) | ||
| .bind(1, "cursor1", Date.now()) | ||
| .run(); |
Contributor
There was a problem hiding this comment.
we have funciton in storage for that. You don't need to make a raw SQL here
Comment on lines
+658
to
+661
| }); | ||
|
|
||
| it("getSignedRedeems should return signed redeems", async () => { | ||
| await insertRedeemRequest(storage, 1, "redeemer1", recipientScript, 3000, 1000, "0xSuiTx1"); |
Contributor
There was a problem hiding this comment.
Suggested change
| }); | |
| it("getSignedRedeems should return signed redeems", async () => { | |
| await insertRedeemRequest(storage, 1, "redeemer1", recipientScript, 3000, 1000, "0xSuiTx1"); |
Contributor
There was a problem hiding this comment.
you can merge the tests
| await insertRedeemRequest(storage, 1, "redeemer1", recipientScript, 3000, 1000, "0xSuiTx1"); | ||
| const redeem = await storage.getRedeemWithSetup(1); | ||
| expect(redeem).not.toBeNull(); | ||
| expect(redeem!.nbtc_pkg).toBe("0xPkg1"); |
Contributor
There was a problem hiding this comment.
same here , you can merge this with the test above
| }, | ||
| ]); | ||
| await storage.updateRedeemInputSig(1, 1, "signId123"); | ||
| const info = await storage.getRedeemInfoBySignId("signId123"); |
Contributor
There was a problem hiding this comment.
you can merge this test with the test above, and run the check just before updateRedeemStatus
| .first<{ status: string }>(); | ||
| expect(redeem!.status).toBe(RedeemRequestStatus.Finalized); | ||
| expect(utxo!.status).toBe(UtxoStatus.Spent); | ||
| }); |
Contributor
There was a problem hiding this comment.
this test can be also merged with one of the tests above
rmeena840
force-pushed
the
348-add-missing-tests-to-sui-indexer-storage
branch
from
ebe72cf to
20d0907
Compare
Signed-off-by: rmeena840 <rmeena840@gmail.com>
rmeena840
force-pushed
the
348-add-missing-tests-to-sui-indexer-storage
branch
from
20d0907 to
cdf6a91
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: #348
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
Add test coverage for additional indexer storage operations related to Sui GraphQL cursors and redeem handling.
Tests: