fix(onchain): remove SorobanOnchainAdapter stub that fabricated success - #449
Merged
kilodesodiq-arch merged 1 commit intoAug 20, 2026
Conversation
SorobanOnchainAdapter posted a hand-rolled {contractId, method, args}
JSON blob to simulateTransaction/sendTransaction — not a Soroban
envelope — and returned status 'success' with an empty transactionHash
for every mutation, silently corrupting the audit trail. Delete it and
its stale coverage-baseline entry, keeping SorobanAdapter as the single
canonical adapter, and add a regression spec proving the adapter
submits real TransactionBuilder-built envelopes and only reports
success with a confirmed non-empty hash.
3 tasks
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.
Summary
Closes #426
SorobanOnchainAdapterimplementedOnchainAdapterbut could never drive theaid_escrowcontract: itsinvokeContractposted a hand-rolled{contractId, method, args}JSON blob tosimulateTransaction/sendTransaction(no XDR envelope, signature, or fee), called non-existent contract methods (initialize,claim_package,disburse_package, a 6-argcreate_package), and returnedstatus: 'success'with an emptytransactionHashfor every mutation regardless of what the RPC returned. It was superseded bySorobanAdapterbut never deleted — exported dead code that reads as a working adapter and silently corrupts the audit trail if selected. This PR deletes it entirely and locks the canonicalSorobanAdapterto the correct behavior with a regression spec.The single most important design decision: delete, don't repair. The class is unreferenced (its only importers are its own file and the coverage baseline), the module already registers
MockOnchainAdapter/SorobanAdapteronly, and the issue's proposed design explicitly names removal as the fix. Keeping two*soroban*.adapter.tsfiles invites re-wiring; the guardrail is now a test that fails if any adapter submits a JSON blob instead of a real envelope.Why
The
OnchainAdapterinterface masks the gap — every method returnsstatus: 'success' | 'failed'and a stringtransactionHash, so a stub satisfies the type system while doing nothing. The old class exploited exactly that:createAidPackage/claimAidPackage/disburseAidPackagereturnedstatus: 'success'andtransactionHash: ''unconditionally, andamountClaimed/amountDisbursedwere hardcoded'0'. Becauseonchain.module.tsnever registered it, the damage was latent — but anyONCHAIN_ADAPTER=soroban-onchain-style config (or a future contributor "fixing" the wiring) would have observed success for every operation while nothing was written on-chain. The existingonchain.module.spec.tsasserted only that an adapter instance was created, not that calls produce real transactions — the regression spec closes that hole.What was built
Deleted:
app/backend/src/onchain/soroban-onchain.adapter.ts— the fabricated adapter. Fully superseded bysoroban.adapter.ts(SorobanAdapter), which builds real envelopes viaTransactionBuilder, signs with the admin keypair, simulates, submits, and polls for confirmation. Deleting it also removes the duplicateexport { ONCHAIN_ADAPTER_TOKEN }re-export.Modified:
app/backend/test/coverage-baseline.json— removed the stale per-file coverage entry for the deleted adapter (regenerating the baseline would produce the same; the entry referenced a file that no longer exists).Added (matching test file):
app/backend/src/onchain/soroban.adapter.spec.ts— 5 regression tests that mock@stellar/stellar-sdk's RPC boundary (rpc.ServerwithsimulateTransaction/sendTransaction/getTransaction,TransactionBuilder,Contract,Keypair) and assert:createAidPackagepasses a realTransactionBuilder-built envelope (an object withtoEnvelope/toXDR) tosimulateTransactionandsendTransaction— the JSON-blob payload from the deleted class would fail these assertions (JSON.stringifyof the payload must not containcontractId/method/args).Contract simulation errorinstead of returningstatus: 'success'.PENDING/DUPLICATErejects.FAILEDtransaction rejects.create_packageentrypoint with a realContract.calloperation (the stub used a 6-arg JSON blob).Acceptance criteria coverage
soroban-onchain.adapter.tsis deleted or reimplemented to sign and submit real Soroban envelopes, and exactly one canonical Soroban adapter remains for thesorobanconfig value. (File deleted;onchain.module.tsregisters onlySorobanAdapterforsoroban, asserted by the existingcreateOnchainAdaptermodule spec.)status: 'success'with an emptytransactionHashafter a failed or never-sent transaction. (soroban.adapter.spec.ts— success is only returned with a 64-char confirmed hash; simulation failure, non-PENDING submission, and FAILED receipt all reject.)simulateTransactionpayload is not a valid Soroban envelope (no{contractId, method, args}JSON blob), and passes forSorobanAdapter. (soroban.adapter.spec.ts— first test asserts the payload is an envelope object withtoEnvelope/toXDRand thatJSON.stringify(payload)contains nocontractId/method/argskeys; the deleted class's payload shape fails this assertion by construction.)Test plan
jest src/onchain src/onchain/utils— 26/26 passing (5 new)jest(full backend unit suite) — 557 passed, 10 skipped, 0 failed (5 new tests for this feature)jest --config ./test/jest-coverage.js --coverage src/onchain/soroban.adapter.spec.ts— 5/5 passing under the CI coverage configeslint(backendeslint.config.mjs) — 0 errors on changed filesprettier --check— cleannest build— succeedsEnv vars / Notes
No new environment variables; no config values change (
sorobanwas and remains the only Soroban adapter value). The deleted class'sSOROBAN_RPC_URL/SOROBAN_CONTRACT_ID/SOROBAN_SECRET_KEY/STELLAR_NETWORKenv vars were never read by anything else —SorobanAdapterusesAID_ESCROW_CONTRACT_ID,STELLAR_RPC_URL,STELLAR_NETWORK_PASSPHRASE,SOROBAN_NETWORK, andSOROBAN_ADMIN_SECRET_KEY— so no deployment config needs to change.