Fixed checkAllowance false-negative - #411
Merged
Merged
Conversation
…heckAllowance
checkAllowance previously swallowed getAllowance RPC/network errors and
returned { hasAllowance: false }. The create-stream flow keys off
hasAllowance to decide whether to prompt an SEP-41 approve(), so a
transient hiccup incorrectly triggered an extra signed transaction and
fee even when allowance was already sufficient.
Return { hasAllowance: undefined, error } on failure so callers can
distinguish 'checked, insufficient' (hasAllowance === false) from
'couldn't check' (hasAllowance === undefined) and retry the read
instead of defaulting to needs-approval. Update AllowanceResult docs
accordingly.
…solate test state soroban.ts was broken since conduit-protocol#379: duplicated poll logic with a stray status check left the file syntactically invalid and pollForConfirmation still returned string instead of InvokeContractResult. Make poll return { hash, returnValue } on SUCCESS and { hash } on pending timeout so create_stream's stream_id surfaces correctly, and clean up the stray block. Also clear the inclusion-fee cache on resetServer / via __clearFeeStatsCache so fee-stats tests don't leak cached p70 across cases. Tests: update soroban-pipeline mocks to expect object shape, fix tokens.test mock type, add checkAllowance RPC-failure distinguishing test, fix stream.test missing-field order, and mock checkRecipientExists in create/page tests with debounce-aware waits.
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.
Fixed checkAllowance false-negative on RPC failure and pushed in 2 batches:
Batch 1 — lib/tokens.ts:94-104,158-184: Changed AllowanceResult.hasAllowance to boolean | undefined with docs: true = sufficient, false = checked insufficient (needs approve()), undefined = couldn't check (transient RPC). catch now returns { hasAllowance: undefined, currentAllowance: 0n, error } instead of false, so callers must check error and retry rather than defaulting to an unnecessary SEP-41 approve() tx. app/create/page.tsx:248-276 already correctly checks gateway.checkAllowance().success before approve() (mirrors #291).
Closes #384
Batch 2 — lib/soroban.ts:122-127,395-392 + tests: Restored broken invokeContract/pollForConfirmation plumbing from #379 (stray status block made file unparsable): pollForConfirmation now returns Promise surfacing returnValue on SUCCESS, __clearFeeStatsCache/resetServer clears fee-stats cache. Fixed lib/soroban-pipeline.test.ts:203-230, lib/tokens.test.ts:105-130 (mock type, added hasAllowance === undefined failure test), lib/stream.test.ts:116-123 (flags-order), app/create/tests/page.test.tsx:45-99 (mock checkRecipientExists + 700 ms debounce wait) — all 411 tests pass.