fix(adapter-stellar): filter infrastructure state changes in mutability detection#355
fix(adapter-stellar): filter infrastructure state changes in mutability detection#355
Conversation
…ty detection Soroban simulation returns TTL-related state changes (contract instance and WASM code entries) for every function invocation, including read-only ones. This caused all functions to be classified as state-modifying, resulting in "No simple view functions found" in ContractStateWidget. Filter out CONTRACT_CODE and CONTRACT_DATA instance key entries before checking for actual storage modifications.
There was a problem hiding this comment.
Pull request overview
Fixes Stellar Soroban state-mutability detection so read-only contract functions are no longer misclassified as state-modifying due to always-present “infrastructure” TTL bump state changes returned by simulation.
Changes:
- Filter
simulateTransaction().stateChangesto ignoreCONTRACT_CODEand contract-instanceCONTRACT_DATAentries before determining mutability. - Improve logging to report total state changes vs storage-relevant changes.
- Add a changeset to publish the adapter fix as a patch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/adapter-stellar/src/query/handler.ts | Filters out TTL-bump-only state changes when detecting whether a function modifies state. |
| .changeset/fix-stellar-state-mutability.md | Releases the fix as a patch for @openzeppelin/ui-builder-adapter-stellar. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nativeToScVal, | ||
| rpc as StellarRpc, | ||
| TransactionBuilder, | ||
| xdr, |
There was a problem hiding this comment.
Adding xdr as a named import from @stellar/stellar-sdk will break existing Vitest mocks that stub that module without providing an xdr export (e.g. packages/adapter-stellar/test/query/sac-query.test.ts). Update the mocks to include an xdr object (or use vi.importActual passthrough) so tests can import this file successfully.
| xdr, |
| // --- Check State Changes --- // | ||
| // Filter out infrastructure state changes that occur for every invocation: | ||
| // 1. CONTRACT_CODE entries (WASM code TTL bumps) | ||
| // 2. CONTRACT_DATA entries with scvLedgerKeyContractInstance key (instance TTL bumps) | ||
| // Only remaining entries represent actual contract storage modifications. | ||
| const storageChanges = (simulationResult.stateChanges ?? []).filter((change) => { | ||
| try { | ||
| const keyType = change.key.switch(); | ||
|
|
||
| if (keyType === xdr.LedgerEntryType.contractCode()) { | ||
| return false; | ||
| } | ||
|
|
||
| if (keyType === xdr.LedgerEntryType.contractData()) { | ||
| const dataKey = change.key.contractData().key(); | ||
| if (dataKey.switch() === xdr.ScValType.scvLedgerKeyContractInstance()) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } catch { | ||
| return true; | ||
| } | ||
| }); | ||
|
|
||
| const hasStorageChanges = storageChanges.length > 0; |
There was a problem hiding this comment.
The new storage-change filtering logic in checkStellarFunctionStateMutability is currently untested. Please add unit tests that simulate stateChanges containing (1) contractCode entries, (2) contractData with scvLedgerKeyContractInstance, and (3) a real contract-data key, and assert the function returns false/true accordingly.
| stateChangesCount: simulationResult.stateChanges?.length || 0, | ||
| modifiesState: Boolean(hasStateChanges), | ||
| totalStateChanges: simulationResult.stateChanges?.length || 0, | ||
| storageChanges: storageChanges.length, |
There was a problem hiding this comment.
In the log payload, storageChanges is a numeric count, but the name reads like it might be the array itself. Consider renaming the field to something like storageChangesCount to avoid confusion in log consumers.
| storageChanges: storageChanges.length, | |
| storageChangesCount: storageChanges.length, |
- Rename storageChanges to storageChangesCount in log payload for clarity - Update sac-query test mock to use importOriginal passthrough for xdr - Add unit tests for state mutability filtering logic covering infrastructure-only changes, mixed changes, empty/undefined stateChanges, and simulation failure fallback
Summary
nonpayable), causing theContractStateWidgetto show "No simple view functions found"stateChanges.length > 0was catching these, marking everything as non-view.CONTRACT_CODEentries andCONTRACT_DATAentries withscvLedgerKeyContractInstancekey before checking for actual storage modificationsTest plan
CDXBAC2SN6DR67PWHP45KIG2A2AY7EC2O2H4IIW4GR3NQP7AY37RKJU5on Stellar Testnet in UI BuilderContractStateWidgetnow shows view functions (e.g.,owner,paused,greeting,get_settings,total_periods)stake,withdraw,set_greeting,initialize) are still correctly classified as non-view