#157 feat(core): decode Stellar Asset Contract (SAC) balances FIXED - #265
Merged
AbdulmalikAlayande merged 2 commits intoJun 29, 2026
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new ChangesSAC Inspect Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@veloura-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/inspect.ts`:
- Around line 21-26: The inspect command is eagerly calling getDatabase() before
it needs any local metadata, which makes `sorokeep inspect` fail in read-only or
unwritable environments. Update the inspect flow in inspect.ts so
inspectContract can be invoked with an optional DB handle, and only open the
SQLite database when a local record is actually needed; keep the existing
fallback behavior to options.network, options.rpcUrl, and the default testnet
path when no DB data is available.
In `@src/core/inspect.ts`:
- Around line 146-147: The network and RPC resolution in inspect should
prioritize explicit CLI input over stored contract metadata. Update the logic in
the inspect flow around the contract lookup so options.network and
options.rpcUrl are used first, with contract?.network and contract?.rpc_url only
as fallbacks, and keep the default to "testnet" when neither is provided. Verify
the behavior in the code that computes network and rpcUrl so a contract existing
in the DB does not override user-supplied --network or --rpc-url values.
- Around line 213-249: Treat decode errors as data issues rather than absence:
in the inspect flow around the foundEntry/parseSacBalance handling, stop
returning a zeroed balance when an entry exists but valXdr is missing or fails
to decode. Update the balance branch so item.type === "balance" either
propagates an error/undecodable state or otherwise preserves that the entry was
found but unreadable, using foundEntry, parseSacBalance, and the returned
balance object to distinguish “not found” from “present but undecodable.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 752343be-9f73-434d-842b-703b846a93c7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
src/commands/inspect.tssrc/core/inspect.tssrc/index.tssrc/lib.tssrc/rpc/client.tstests/commands/inspect.test.tstests/core/inspect.test.tstests/lib.test.ts
📜 Review details
🔇 Additional comments (2)
src/rpc/client.ts (1)
43-46: LGTM!Also applies to: 268-328
src/core/inspect.ts (1)
1-129: LGTM!Also applies to: 152-210, 250-283
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | 3dd17ce | tests/core/vault.test.ts | View secret |
| - | - | Generic High Entropy Secret | 3dd17ce | tests/core/vault.test.ts | View secret |
| - | - | Generic High Entropy Secret | 3dd17ce | tests/core/vault.test.ts | View secret |
| - | - | Generic High Entropy Secret | b59bef4 | tests/core/vault.test.ts | View secret |
| - | - | Generic High Entropy Secret | b59bef4 | tests/core/vault.test.ts | View secret |
| - | - | Generic High Entropy Secret | b59bef4 | tests/core/vault.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
…t-Contract-(SAC)-balances-FIX
Contributor
Author
|
@AbdulmalikAlayande please review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rpc/client.ts (1)
302-303: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWrap these RPC calls in
executeWithRetry.Both new inspect paths bypass the retry wrapper, so a single timeout/429/5xx aborts
inspectinstead of using the backoff policy added for RPC calls.Suggested fix
- const response = await this.server.getLedgerEntries(...keys); + const response = await executeWithRetry(() => this.server.getLedgerEntries(...keys)); ... - const sim = await this.server.simulateTransaction(tx); + const sim = await executeWithRetry(() => this.server.simulateTransaction(tx));Also applies to: 342-342
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/rpc/client.ts` around lines 302 - 303, The new inspect RPC paths in the client currently call this.server.getLedgerEntries directly and bypass the retry policy, so wrap these calls in executeWithRetry just like the other RPC methods. Update the inspect flow in client.ts around the getLedgerEntries-based logic (and the other affected inspect path noted in the review) so transient timeout/429/5xx failures are retried with the existing backoff behavior rather than failing immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/rpc/client.ts`:
- Around line 302-303: The new inspect RPC paths in the client currently call
this.server.getLedgerEntries directly and bypass the retry policy, so wrap these
calls in executeWithRetry just like the other RPC methods. Update the inspect
flow in client.ts around the getLedgerEntries-based logic (and the other
affected inspect path noted in the review) so transient timeout/429/5xx failures
are retried with the existing backoff behavior rather than failing immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f5f99294-ff80-45d3-b84e-8cf16563713c
📒 Files selected for processing (3)
src/index.tssrc/lib.tssrc/rpc/client.ts
📜 Review details
⚠️ CI failures not shown inline (1)
GitHub Check: GitGuardian Security Checks: 3 secrets uncovered!
Conclusion: failure
#### 3 secrets were uncovered from the scan of 43 commits in your pull request. ❌
Please have a look to GitGuardian findings and remediate in order to secure your code.
Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.
### 🔎 Detected hardcoded secrets in your pull request
- Pull request `#289`: `feat/issue-110-webhook-delivery-channel` 👉 `main` (1 commit)
- Pull request `#288`: `feat/issue-115-extend-footprint-ttl-op` 👉 `main` (1 commit)
- Pull request `#287`: `feat/issue-119-restore-command` 👉 `main` (1 commit)
- Pull request `#286`: `feat/cli-custom-schema` 👉 `main` (1 commit)
- Pull request `#285`: `feat/scval-json-translator` 👉 `main` (1 commit)
- Pull request `#283`: `feat/alerts-add-list-remove-tests` 👉 `main` (2 commits)
- Pull request `#282`: `feat/auto-extension-monitor-integration` 👉 `main` (1 commit)
- Pull request `#281`: `feat/alerts-test-command` 👉 `main` (1 commit)
- Pull request `#272`: `main` 👉 `main` (2 commits)
- Pull request `#271`: `feature/dynamic-key-discovery` 👉 `main` (1 commit)
- Pull request `#270`: `#145-feat(core)--integrate-HashiCorp-Vault-for-key-retrieval-FIX` 👉 `main` (1 commit)
- Pull request `#269`: `feat/simulation-traffic-cache` 👉 `main` (5 commits)
- Pull request `#268`: `feat/secure-keychain-storage` 👉 `main` (3 commits)
- Pull request `#267`: `feat/instance-storage-scanner` 👉 `main` (2 commits)
- Pull request `#266`: `feat/budget-exhaustion-alerts` 👉 `main` (1 commit)
- Pull request `#265`: `#157-feat(core)--decode-Stellar-Asset-Contract-(SAC)-balances-FIX` 👉 `main` (2 commits)
- Pull request `#263`: `fix/issue-137-simulation-failures` 👉 `main` (1 commit)
- Pull request `#261`: `feat-poll-interval-overrides` 👉 `main` (2 commits)
- Pull request `#256`: `feat/154-ledger-key-...
🔇 Additional comments (4)
src/index.ts (1)
15-15: LGTM!Also applies to: 43-45
src/lib.ts (1)
19-23: LGTM!src/rpc/client.ts (2)
15-41: LGTM!
72-72: LGTM!
AbdulmalikAlayande
pushed a commit
that referenced
this pull request
Aug 2, 2026
AbdulmalikAlayande
pushed a commit
that referenced
this pull request
Aug 2, 2026
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.
FINDINGS
DataKey::Balance(Address). When serialized to XDR (xdr.ScVal), this enum variant is represented as anscvVeccontaining two elements:Symbol("Balance")andAddress(address).i128), built-in Soroban SAC instances (soroban-env-host) store address balances as Soroban maps (scvMap). This map layout contains key-value pairs for:amount(stored as ani128integer)authorized(stored as aboolflag indicating whether the trustline/account is authorized to transact)clawback(stored as aboolflag indicating whether clawback is enabled)contractExecutableStellarAsset. Checking this property viagetContractInstanceEntryallowssorokeepto isolate standard SAC contracts and fail gracefully when attempting to run SAC inspections on non-SAC contracts (e.g., customcontractExecutableWasmcontracts).sorokeep's RPC client (getEntryTTLs) stripped out raw XDR values (entry.val) from responses to keep application models lightweight. To inspect token balances without modifying existing TTL architectures, a dedicated storage entry query method (getContractStorageEntries) was required.2. Fix Features
Custom SAC Layout Parser (
parseSacBalance):src/core/inspect.tsthat takes raw on-chain base64 XDR storage values (valXdr) and safely unpacks the Soroban map layout (scvMap).amount(handling 128-bit signed integer decoding),authorizedstatus, andclawbackstatus into clean JavaScript/TypeScript records.Balance Slot Auto-Locator (
buildSacBalanceKeyXdr):G...or contractC...).LedgerKeyContractDataXDR string forDataKey::Balance(Address), eliminating the need for developers to manually craft base64 XDR keys.Dynamic Decimals Decoder & Precision Formatter (
formatTokenBalance&getSacDecimals):getSacDecimalstoStellarRpcClientto dynamically inspect token decimals on-chain via simulation of thedecimals()view method (with safe fallback to standard 7 decimals).formatTokenBalanceusing bigint arithmetic to convert raw stroops into exact decimal strings (e.g.,10500000stroops with 7 decimals ->"1.05"), avoiding JavaScript floating-point rounding inaccuracies.Graceful Failure Guard on Non-SAC Contracts (
inspectContract):contractExecutableStellarAsset, the inspection fails gracefully with an explicit error message ("Contract ... is not a standard Stellar Asset Contract (SAC)") rather than crashing or throwing unhandled XDR decoding exceptions.CLI Shortcut Command (
sorokeep inspect):sorokeep inspect <contractId>command to the CLI with the--entryoption.--entry balance:<address>shortcut to auto-locate balance storage slots and render rich terminal outputs detailing decoded balances, authorization flags, clawback settings, and live entry TTL ledgers.Public Programmatic Library Surface (
src/lib.ts):inspectContract,parseSacBalance,buildSacBalanceKeyXdr,formatTokenBalance, and all associated TypeScript interfaces insrc/lib.tsso external Node.js tools and scripts can leverage the SAC inspection engine directly.CLOSE #157