Conversation
ChainLearnOfficial#442 already added stellar-contracts-env.test.ts for getContractAddress's lazy env resolution. The rest of lib/stellar/contracts.ts — the actual read/write helpers ChainLearnOfficial#359 asks for — had no tests. Adds 20 in stellar-contracts-helpers.test.ts, mocking lib/stellar/transactions and using @stellar/stellar-sdk's own nativeToScVal to build real ScVal XDR fixtures rather than hand-rolled stand-ins, so decodeFirstSimResult's actual decode path is exercised. Covers getContractBalance/readRewardBalance (incl. values beyond Number.MAX_SAFE_INTEGER, missing-config and RPC-rejection errors), decodeFirstSimResult's error paths and the bare-array response shape, verifyCredentialOnChain, getProgressOnChain, and that claimRewardOnChain/mintCredentialOnChain delegate to signAndSubmitTransaction and pass a failed result through unchanged. Writing these tests surfaced three behaviors worth a maintainer's attention, each documented with a dedicated test and NOT fixed here (test-only PR): 1. verifyCredentialOnChain fails open: Boolean(record.valid ?? true) treats an absent `valid` field the same as an explicit `true`. 2. verifyCredentialOnChain silently drops issuedAt when the contract encodes issued_at as u64 rather than u32 — Soroban decodes u32 to a JS number but u64/i128 to a bigint, and the `typeof === "number"` guard only matches the former. u64 is a plausible choice for a Unix timestamp (u32 overflows in 2106). 3. readCredentialMetadata returns simulateContractCall's raw, un-decoded RPC response (`{ results: [{ xdr }] }`) instead of the decoded metadata object every sibling helper produces via decodeFirstSimResult — likely broken for any real caller. Verified: 20/20 new tests pass; eslint and tsc --noEmit report nothing for the new file; full suite before/after is byte-identical in which 20 files/50 tests fail (confirmed via git stash) — pre-existing on main, unrelated to this change. Refs ChainLearnOfficial#359
❌ Deploy Preview for chainlearn failed.
|
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
2 participants
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.
What this is
Test-only. No production code changed. Following up on my note in #459: #442 already covers
getContractAddress's lazy env resolution; the actual read/write helpers #359 asks for (getContractBalance,verifyCredentialOnChain,getProgressOnChain, plusclaimRewardOnChain/mintCredentialOnChain/readCredentialMetadata) had none. Adds 20 tests insrc/tests/shared/stellar-contracts-helpers.test.ts.Since the helpers already existed, this uses
Refs #359, notCloses.Approach
Mocks
lib/stellar/transactions(simulateContractCall,signAndSubmitTransaction) at the boundary, but builds simulation-result fixtures with@stellar/stellar-sdk's ownnativeToScVal(...).toXDR("base64")rather than hand-rolled strings — sodecodeFirstSimResult's real base64 XDR decode path is what's under test, not a stand-in that merely looks right.verifyCredentialOnChainfails open.Boolean(record.valid ?? true)treats an absentvalidfield the same as an explicittrue. If the contract only setsvalidon failure (or omits it in some path), every such credential verifies as valid.issuedAtis silently dropped for a u64issued_at. Verified against the real SDK:scValToNativedecodes a u32 to a JSnumberbut a u64/i128 to abigint. The function'stypeof record.issued_at === "number"guard only matches the former, so a perfectly valid u64 timestamp — a plausible choice, since u32 overflows in 2106 — silently producesissuedAt: undefined.readCredentialMetadatareturns the raw, un-decoded RPC response, not decoded metadata. Every sibling helper passes its result throughdecodeFirstSimResult; this one returnssimulateContractCall's return value cast straight toRecord<string, unknown>— the caller gets{ results: [{ xdr: "<base64>" }] }, not metadata fields. This looks like it would break any real caller.Each is backed by its own test (search the diff for
KNOWN BUG), so they're reproducible, not just a claim in this description.Verification
eslintandtsc --noEmitreport nothing for the new file.git stash) — pre-existing onmain, unrelated. With this file: 429 passing vs 409 onmain(the 20 new tests).Refs #359