Repository navigation
chore: regenerate package-lock.json and update dependencies - #216
Conversation
- Add esbuild platform-specific binaries for cross-platform support - Update @emnapi core, runtime, and wasi-threads dependencies - Sync extension.ts config loading with updated RPC client interface - Update RPC client to handle improved error handling and connection logic - Refine config utility for consistent parameter handling - Update extension tests to reflect API changes - Ensure npm lockfile consistency across environments
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📜 Recent review details🔇 Additional comments (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds fee-bump sponsorship to TTL extension flows. Config can now load a ChangesFee-bump sponsor support for TTL extensions
Sequence Diagram(s)sequenceDiagram
participant Caller
participant runAutoExtensions
participant extendEntries
participant StellarRpcClient
participant Soroban RPC
Caller->>runAutoExtensions: run(db, network, rpcUrl, sponsorSecret)
runAutoExtensions->>extendEntries: extend(..., sponsorSecret)
alt sponsorSecret provided
extendEntries->>StellarRpcClient: submitExtensionWithFeeBump(...)
StellarRpcClient->>Soroban RPC: simulateTransaction(inner tx)
Soroban RPC-->>StellarRpcClient: resource parameters
StellarRpcClient->>Soroban RPC: sendTransaction(fee-bump tx)
Soroban RPC-->>StellarRpcClient: tx hash
StellarRpcClient->>Soroban RPC: pollTransaction(tx hash)
Soroban RPC-->>StellarRpcClient: terminal status
else no sponsorSecret
extendEntries->>StellarRpcClient: submitExtension(...)
StellarRpcClient->>Soroban RPC: sendTransaction(standard tx)
end
extendEntries-->>runAutoExtensions: ExtensionResult
runAutoExtensions-->>Caller: AutoExtensionResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
|
@Arome8240 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! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/core/extension.ts`:
- Around line 157-164: The fee-bump branch in extension.ts is passing
sponsorSecret directly into submitExtensionWithFeeBump(), which breaks
env:VAR_NAME support because Keypair.fromSecret() receives the unresolved
string. Resolve sponsorSecret with resolveSecretKey() before entering the
fee-bump path, using the same error handling as the main secret resolution so
unresolved values return a normal extension error. Keep the existing
submitExtensionWithFeeBump and submitExtension call flow, but ensure the
resolved sponsor secret is what gets forwarded.
In `@src/rpc/client.ts`:
- Around line 377-383: The fee-bump construction in
`assembleTransaction`/`buildFeeBumpTransaction` is using `assembledInner.fee`,
which already includes the Soroban resource fee and causes the sponsor to be
overcharged. Update the fee-bump call in `src/rpc/client.ts` to pass the classic
base fee used by the inner transaction or network base fee instead of
`assembledInner.fee`, keeping the sponsor fee calculation separate from the
assembled transaction’s resource fee.
In `@src/utils/config.ts`:
- Around line 55-66: Validate the fee sponsor secret while building the config
object in the config parsing flow so non-string YAML values are rejected early.
In the return object from the YAML.parse handling in config.ts, update the
feeSponsorSecret assignment to only pass through values when
parsed.feeSponsorSecret is a string, otherwise fall back to an undefined/default
value like the other guarded fields. This change should be applied in the config
loader path that returns SorokeepConfig so later consumers such as
Keypair.fromSecret do not receive invalid non-string input.
- Around line 88-89: The config write path in config.ts only sets permissions
when the file is created, so existing config.yaml files may keep broader access
after secrets like feeSponsorSecret are written. Update the config persistence
flow around the YAML.stringify and fs.writeFileSync call to also apply a
best-effort fs.chmodSync(configPath, 0o600) after writing so previously created
files are tightened too. Use the existing config save logic in config.ts to
place this immediately after the write while keeping the current behavior intact
on unsupported platforms.
In `@stdout`:
- Around line 53-78: The stdout snapshot includes machine-specific volatile data
that should be stabilized before committing. Update the fixture output to redact
or normalize the hostname, pid, and absolute stack-trace paths so the snapshot
does not depend on the local environment. Keep the semantic log content from
MonitorCycle intact, but make the generated stdout deterministic by adjusting
the snapshot source where the logs are captured or serialized.
In `@tests/core/extension.test.ts`:
- Around line 21-37: The current tests mock StellarRpcClient end-to-end, so they
only validate call routing and miss the real fee-bump assembly path. Add a
client-level test around submitExtensionWithFeeBump that uses the actual
implementation instead of the mocked submitExtensionWithFeeBump/submitExtension
methods, then assert the returned envelope is sponsor-wrapped correctly with the
expected wrapper fee and sponsor signature. Keep the existing mocked tests for
routing, but extend coverage in tests/core/extension.test.ts using the real
extendEntries/submitExtensionWithFeeBump path so regressions in fee-bump
construction are caught.
🪄 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: 5c5aa3f3-fec1-4464-add1-fd59f27b4011
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
src/core/extension.tssrc/rpc/client.tssrc/utils/config.tsstdouttests/core/extension.test.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
src/utils/config.ts
[warning] 53-53: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(configPath, "utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 88-88: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(configPath, yamlStr, { encoding: "utf-8", mode: 0o600 })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
| const txResult = sponsorSecret | ||
| ? await client.submitExtensionWithFeeBump( | ||
| entryKeyXdrs, | ||
| extendToLedgers, | ||
| secretKey, | ||
| sponsorSecret, | ||
| ) | ||
| : await client.submitExtension(entryKeyXdrs, extendToLedgers, secretKey); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the sponsor secret before calling the fee-bump path.
feeSponsorSecret is documented as supporting env:VAR_NAME, but this branch forwards the raw string straight into submitExtensionWithFeeBump(). That reaches Keypair.fromSecret(...), so a configured value like env:FEE_SPONSOR_SECRET fails instead of loading the environment variable. Run sponsorSecret through resolveSecretKey() first and return a normal extension error if it cannot be resolved.
Suggested fix
- const txResult = sponsorSecret
+ const resolvedSponsorSecret = sponsorSecret
+ ? resolveSecretKey(sponsorSecret)
+ : null;
+
+ if (sponsorSecret && !resolvedSponsorSecret) {
+ return {
+ success: false,
+ contractId,
+ entriesExtended: 0,
+ error: `Cannot resolve fee sponsor from source "${sponsorSecret}"`,
+ };
+ }
+
+ const txResult = resolvedSponsorSecret
? await client.submitExtensionWithFeeBump(
entryKeyXdrs,
extendToLedgers,
secretKey,
- sponsorSecret,
+ resolvedSponsorSecret,
)
: await client.submitExtension(entryKeyXdrs, extendToLedgers, secretKey);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const txResult = sponsorSecret | |
| ? await client.submitExtensionWithFeeBump( | |
| entryKeyXdrs, | |
| extendToLedgers, | |
| secretKey, | |
| sponsorSecret, | |
| ) | |
| : await client.submitExtension(entryKeyXdrs, extendToLedgers, secretKey); | |
| const resolvedSponsorSecret = sponsorSecret | |
| ? resolveSecretKey(sponsorSecret) | |
| : null; | |
| if (sponsorSecret && !resolvedSponsorSecret) { | |
| return { | |
| success: false, | |
| contractId, | |
| entriesExtended: 0, | |
| error: `Cannot resolve fee sponsor from source "${sponsorSecret}"`, | |
| }; | |
| } | |
| const txResult = resolvedSponsorSecret | |
| ? await client.submitExtensionWithFeeBump( | |
| entryKeyXdrs, | |
| extendToLedgers, | |
| secretKey, | |
| resolvedSponsorSecret, | |
| ) | |
| : await client.submitExtension(entryKeyXdrs, extendToLedgers, secretKey); |
🤖 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/core/extension.ts` around lines 157 - 164, The fee-bump branch in
extension.ts is passing sponsorSecret directly into
submitExtensionWithFeeBump(), which breaks env:VAR_NAME support because
Keypair.fromSecret() receives the unresolved string. Resolve sponsorSecret with
resolveSecretKey() before entering the fee-bump path, using the same error
handling as the main secret resolution so unresolved values return a normal
extension error. Keep the existing submitExtensionWithFeeBump and
submitExtension call flow, but ensure the resolved sponsor secret is what gets
forwarded.
| // Wrap in a fee-bump transaction — sponsor pays the base fee | ||
| const feeBump = TransactionBuilder.buildFeeBumpTransaction( | ||
| sponsorKeypair, | ||
| assembledInner.fee, | ||
| assembledInner, | ||
| passphrase, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant implementation and related usages
git ls-files | rg '^src/rpc/client\.ts$|buildFeeBumpTransaction|assembleTransaction|minResourceFee|fee-bump|feeBump' -n
# Show the relevant section around the cited lines
sed -n '330,420p' src/rpc/client.ts
# Search for other fee-bump constructions in the repo
rg -n "buildFeeBumpTransaction\(|assembleTransaction\(|minResourceFee|sponsor.*fee|fee bump" srcRepository: AbdulmalikAlayande/sorokeep
Length of output: 3909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect any local docs or vendored references if present
rg -n "buildFeeBumpTransaction|fee-bump|base fee|resourceFee|minResourceFee" .Repository: AbdulmalikAlayande/sorokeep
Length of output: 1149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If package metadata is available, inspect the declared stellar-sdk version
git ls-files | rg '^(package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock|npm-shrinkwrap\.json)$' -nRepository: AbdulmalikAlayande/sorokeep
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect package metadata for stellar SDK version(s)
sed -n '1,220p' package.json
printf '\n--- package-lock references ---\n'
rg -n '"`@stellar/stellar-sdk`"|stellar-base|stellar-sdk' package-lock.jsonRepository: AbdulmalikAlayande/sorokeep
Length of output: 2144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any inline docs or comments around the helper that may specify fee semantics
sed -n '300,390p' src/rpc/client.ts
sed -n '90,140p' src/core/extension.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 4181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the package version actually used for the Stellar SDK dependency
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(pkg.dependencies?.['`@stellar/stellar-sdk`'] || pkg.devDependencies?.['`@stellar/stellar-sdk`'] || 'missing');
JS
# Extract the lockfile entry if present
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path('package-lock.json').read_text())
pkgs = lock.get('packages', {})
for k, v in pkgs.items():
if k.endswith('node_modules/@stellar/stellar-sdk') or k.endswith('node_modules/stellar-sdk') or k.endswith('node_modules/@stellar/stellar-base'):
print(k, v.get('version'))
PYRepository: AbdulmalikAlayande/sorokeep
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' package.json
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path('package-lock.json').read_text())
for name, meta in lock.get('packages', {}).items():
if name.endswith('node_modules/@stellar/stellar-sdk') or name.endswith('node_modules/stellar-sdk') or name.endswith('node_modules/@stellar/stellar-base'):
print(name, meta.get('version'))
PYRepository: AbdulmalikAlayande/sorokeep
Length of output: 1645
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '560,820p' tests/core/extension.test.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 6753
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the Stellar SDK version used by this repo
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log('dependency', pkg.dependencies?.['`@stellar/stellar-sdk`'] || pkg.devDependencies?.['`@stellar/stellar-sdk`'] || 'missing');
JS
# If the lockfile is v2/v3, print the installed package version entries
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path('package-lock.json').read_text())
for path, meta in lock.get('packages', {}).items():
if path.endswith('node_modules/@stellar/stellar-sdk') or path.endswith('node_modules/stellar-sdk'):
print(path, meta.get('version'))
PYRepository: AbdulmalikAlayande/sorokeep
Length of output: 225
🌐 Web query:
Stellar JS SDK buildFeeBumpTransaction fee parameter base fee resourceFee assembleTransaction assembledInner.fee semantics
💡 Result:
In the Stellar JavaScript SDK, the FeeBumpTransaction is used to increase the fee of an existing transaction to facilitate its inclusion in the ledger [1][2][3]. buildFeeBumpTransaction Fee Parameter The fee parameter in buildFeeBumpTransaction represents the total fee (in stroops) that the fee-paying account is willing to pay for the entire fee-bump transaction [2][3]. Crucially, this fee must satisfy two requirements: it must be greater than or equal to the network minimum fee for the total number of operations (inner transaction operations + 1 for the fee-bump operation), and it must be greater than or equal to the fee already specified in the inner transaction [3]. SDK documentation recommends that the fee-bump amount be at least 10 times the original inner transaction fee [2]. baseFee and resourceFee When building a fee-bump transaction, the SDK calculates the necessary fee based on the inner transaction's characteristics [2]. - baseFee: This parameter, provided to the builder, typically refers to the maximum fee the submitter is willing to pay per operation [1][2]. - resourceFee: This is a specialized fee component specifically for Soroban (smart contract) transactions [2]. The SDK's TransactionBuilder inspects the inner transaction to identify Soroban data. If present, it extracts the resourceFee to ensure the fee-bump calculation correctly isolates the "inclusion fee" from the Soroban resource fees, preventing double-counting or incorrect fee assessments [2][4]. The inclusion fee is calculated as the total inner transaction fee minus the resource fee, divided by the number of operations [2]. assembleTransaction and assembledInner.fee The assembleTransaction utility (often used in contexts involving Soroban RPC) is designed to correctly construct a transaction by merging simulation results with user-defined parameters [4]. - The assembledInner.fee represents the calculated classic fee for the transaction being assembled. - The semantics of this fee handling involve adjusting the fee to account for the sum of the classic fee and the resource fee provided by the network during simulation [4]. Specifically, the builder clones the existing transaction and sets the fee to ensure the resulting transaction covers both the classic operation costs and the required Soroban resource fees [4]. These mechanisms collectively ensure that transactions—whether classic payment operations or complex smart contract invocations—have sufficient fees to be prioritized by validators without inadvertently overpaying due to overlapping fee types [1][2][4].
Citations:
- 1: https://developers.stellar.org/docs/learn/fundamentals/fees-resource-limits-metering
- 2: https://stellar.github.io/js-stellar-sdk/js-stellar-base_src_transaction_builder.js.html
- 3: https://developers.stellar.org/docs/build/guides/transactions/fee-bump-transactions
- 4: https://stellar.github.io/js-stellar-sdk/lib_rpc_transaction.js.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the Stellar SDK version used by this repo
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log('dependency', pkg.dependencies?.['`@stellar/stellar-sdk`'] || pkg.devDependencies?.['`@stellar/stellar-sdk`'] || 'missing');
JS
# If the lockfile is v2/v3, print the installed package version entries
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path('package-lock.json').read_text())
for path, meta in lock.get('packages', {}).items():
if path.endswith('node_modules/@stellar/stellar-sdk') or path.endswith('node_modules/stellar-sdk'):
print(path, meta.get('version'))
PYRepository: AbdulmalikAlayande/sorokeep
Length of output: 225
🌐 Web query:
site:stellar.github.io js-stellar-base TransactionBuilder.buildFeeBumpTransaction source code fee resourceFee inner fee
💡 Result:
In the Stellar SDKs, fee bump transactions are not directly constructed via a method named buildFeeBumpTransaction on the TransactionBuilder class. Instead, fee bump transactions are handled using the FeeBumpTransaction class [1]. The construction and management of transactions—including the orchestration of fees for Soroban (smart contract) operations—involve the following components: 1. FeeBumpTransaction Class: This class is used to wrap an existing transaction (the "inner" transaction) to create a fee-bumped transaction [1]. It holds the fee source and the total fee paid to the network [1]. The inner transaction is the original transaction whose fees are being covered [1]. 2. TransactionBuilder: This class is used to build standard Stellar transactions [2]. For operations involving Soroban, resource management (like resource fees and CPU/memory usage) is handled through the sorobanData parameter in the TransactionBuilder options, which accepts an xdr.SorobanTransactionData instance [3]. 3. Resource Fees: When working with Soroban, you typically do not calculate these manualy; instead, the server's prepareTransaction method is used [4]. This method simulates the contract invocation, calculates the required resource fees and footprint, and updates the transaction with the necessary data [4]. The transaction fee is then automatically padded with the minimum resource fees discovered during the simulation [4]. In summary, rather than building a fee-bump transaction manually via a builder method, developers typically use TransactionBuilder to define the operations [2], the server's prepareTransaction to calculate resource and inner transaction fees [4], and finally wrap the transaction in a FeeBumpTransaction if a separate fee payer is required [1].
Citations:
- 1: https://stellar.github.io/js-stellar-sdk/FeeBumpTransaction.html
- 2: https://stellar.github.io/js-stellar-base/TransactionBuilder.html
- 3: https://stellar.github.io/js-stellar-base/Operation.html
- 4: https://stellar.github.io/js-soroban-client/Server.html
🌐 Web query:
"buildFeeBumpTransaction" "resourceFee" "inclusion fee" Stellar SDK source
💡 Result:
In Stellar’s JavaScript SDK (js-stellar-base), buildFeeBumpTransaction handles Soroban “resourceFee” separately from the network “inclusion fee” when calculating the outer fee bump’s per-operation fee. 1) Where resourceFee comes from In TransactionBuilder.buildFeeBumpTransaction, the code inspects the inner transaction envelope. If the envelope is a transaction (EnvelopeTypeTx), it reads sorobanData.resourceFee from env.v1.tx.ext.value.resourceFee and stores it as resourceFee [1]. 2) How inclusion fee is derived After extracting resourceFee, it computes innerInclusionFee by taking the inner transaction’s declared fee (innerTx.fee), subtracting the extracted resourceFee, and then dividing by the number of operations in the inner transaction (innerOps) [1]. Conceptually: innerInclusionFee = (innerTx.fee - resourceFee) / innerOps [1] 3) Why this matters for fee bumps The code comment states the intent: “We only want the fee-bump requirement to match the inclusion fee, not the inclusion+resource fee.” [1] That is, the fee-bump bid logic should be based on the inclusion portion (fee bid to get included), excluding the Soroban resource portion. 4) Official docs on what fee bumps bid for (“inclusion” behavior) Stellar’s fee-bump transaction guide describes fee-bump transactions as resubmitting with a higher fee so the transaction can be included despite insufficient fees, and notes replace-by-fee behavior (e.g., later fee bid must be sufficiently higher) [2] [3]. While the guide doesn’t use the literal term “inclusion fee,” it documents the same concept: the outer fee bid is what affects inclusion in the ledger [2][3]. So, matching your query terms: - buildFeeBumpTransaction uses sorobanData.resourceFee (the Soroban resource component) [1]. - It derives the inner transaction’s “inclusion fee” component by subtracting that resourceFee from innerTx.fee and then dividing by inner operation count [1].
Citations:
- 1: https://stellar.github.io/js-stellar-sdk/js-stellar-base_src_transaction_builder.js.html
- 2: https://developers.stellar.org/docs/learn/fundamentals/fees-resource-limits-metering
- 3: https://developers.stellar.org/docs/build/guides/transactions/fee-bump-transactions
🌐 Web query:
site:developers.stellar.org fee-bump transactions inclusion fee resource fee buildFeeBumpTransaction
💡 Result:
In the context of the Stellar network, a fee-bump transaction is a specialized transaction type that allows an account (the fee sponsor) to pay the transaction fees for an existing transaction (the inner transaction) without requiring the original transaction to be re-signed or its sequence numbers modified [1][2]. This mechanism is primarily used to increase the fee of a pending transaction to ensure its inclusion in a transaction set [1][3]. A fee-bump transaction consists of two primary components: 1. The inner transaction envelope: The original transaction that has already been signed [1]. 2. The outer transaction envelope: The wrapper containing the fee-bump details and the signature of the account paying the fee [1]. The buildFeeBumpTransaction function is a method provided by the Stellar SDK used to construct these transactions [3]. When using this function, you typically specify the fee-paying account, the new maximum fee (which must be higher than the fee in the original transaction), and the original transaction envelope [3]. Once constructed, the fee-bump transaction must be signed by the fee-paying account before submission to the network [1][3]. When a fee-bump transaction is submitted for a transaction that is already in the transaction queue, it effectively replaces the original transaction, provided that the new fee meets the network's requirements for replacement (typically a significant increase, such as 10x) [3]. The sequence number remains tied to the source account of the inner transaction, but the fee is debited from the account specified in the fee-bump envelope [1].
Citations:
- 1: https://developers.stellar.org/docs/build/guides/transactions/fee-bump-transactions
- 2: https://developers.stellar.org/docs/build/guides/transactions/path-payments
- 3: https://developers.stellar.org/docs/learn/fundamentals/fees-resource-limits-metering
🌐 Web query:
"js-stellar-sdk" "buildFeeBumpTransaction" "function" "resourceFee" "inclusion fee" "assembledInner.fee"
💡 Result:
In @stellar/stellar-sdk, TransactionBuilder.buildFeeBumpTransaction computes and uses assembledInner.fee-related data as follows: 1) It initializes resourceFee by reading Soroban resourceFee from the inner transaction’s XDR envelope. - The function declares resourceFee as a BigNumber initialized to 0. [1] - It then inspects innerTx.toEnvelope, and only in the case where the envelope is a normal transaction (EnvelopeTypeTx) does it extract Soroban extension data and set resourceFee from sorobanData.resourceFee, defaulting to 0 if absent. [1] 2) It then derives the inner transaction’s “inclusion fee” by subtracting resourceFee from the inner transaction’s total fee and dividing by the number of inner operations. - It computes innerInclusionFee = (innerTx.fee - resourceFee) / innerOps. [1] 3) Where assembledInner.fee fits conceptually - In this code path, the “assembledInner.fee” value you’re looking for is effectively the assembled inner transaction fee that’s represented in innerTx.fee, while resourceFee is separately extracted from the inner transaction’s Soroban extension and removed to get the inclusion-fee portion. [1] Concrete mapping to your terms: - assembledInner.fee (innerTx.fee) → the fee total on the inner transaction object passed into buildFeeBumpTransaction. [1] - resourceFee → computed from inner transaction SorobanData.resourceFee extracted from innerTx.toEnvelope (default 0 if not present). [1] - “inclusion fee” → innerInclusionFee = (innerTx.fee - resourceFee) / innerOps. [1] If you share the exact SDK snippet/version where you see assembledInner.fee, I can map it line-by-line to the specific internal structures in that release, but the above is the authoritative logic inside buildFeeBumpTransaction. [1]
Citations:
Pass the base fee here, not assembledInner.fee. assembleTransaction() already folds the Soroban resource fee into assembledInner.fee, while buildFeeBumpTransaction() derives the fee-bump amount from the inclusion fee separately. Reusing the assembled fee charges the sponsor for the resource fee twice; use the same classic base fee as the inner transaction (or the network base fee) instead.
🤖 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 377 - 383, The fee-bump construction in
`assembleTransaction`/`buildFeeBumpTransaction` is using `assembledInner.fee`,
which already includes the Soroban resource fee and causes the sponsor to be
overcharged. Update the fee-bump call in `src/rpc/client.ts` to pass the classic
base fee used by the inner transaction or network base fee instead of
`assembledInner.fee`, keeping the sponsor fee calculation separate from the
assembled transaction’s resource fee.
| vi.mock("../../src/rpc/client.js", () => { | ||
| return { | ||
| StellarRpcClient: class MockStellarRpcClient { | ||
| constructor() {} | ||
| submitExtension = mockSubmitExtension; | ||
| submitRestore = mockSubmitRestore; | ||
| getEntryTTLs = mockGetEntryTTLs; | ||
| getCurrentLedger = mockGetCurrentLedger; | ||
| simulateExtension = mockSimulateExtension; | ||
| }, | ||
| }; | ||
| return { | ||
| StellarRpcClient: class MockStellarRpcClient { | ||
| constructor() {} | ||
| submitExtension = mockSubmitExtension; | ||
| submitExtensionWithFeeBump = mockSubmitExtensionWithFeeBump; | ||
| submitRestore = mockSubmitRestore; | ||
| getEntryTTLs = mockGetEntryTTLs; | ||
| getCurrentLedger = mockGetCurrentLedger; | ||
| simulateExtension = mockSimulateExtension; | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| // Import after mocking | ||
| const { extendEntries, restoreEntries, simulateExtension, runAutoExtensions } = await import( | ||
| "../../src/core/extension.js" | ||
| ); | ||
| const { extendEntries, restoreEntries, simulateExtension, runAutoExtensions } = | ||
| await import("../../src/core/extension.js"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
The new coverage never validates the real fee-bump envelope.
Because StellarRpcClient is fully mocked, these tests only prove routing. They do not verify the linked issue’s acceptance criterion that the sponsor-wrapped transaction is actually assembled and signed correctly, so bugs in submitExtensionWithFeeBump() can still ship green. Please add at least one client-level test that exercises the real fee-bump construction and asserts the sponsor signature and wrapper fee are correct.
Also applies to: 585-786
🤖 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 `@tests/core/extension.test.ts` around lines 21 - 37, The current tests mock
StellarRpcClient end-to-end, so they only validate call routing and miss the
real fee-bump assembly path. Add a client-level test around
submitExtensionWithFeeBump that uses the actual implementation instead of the
mocked submitExtensionWithFeeBump/submitExtension methods, then assert the
returned envelope is sponsor-wrapped correctly with the expected wrapper fee and
sponsor signature. Keep the existing mocked tests for routing, but extend
coverage in tests/core/extension.test.ts using the real
extendEntries/submitExtensionWithFeeBump path so regressions in fee-bump
construction are caught.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 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. |
|
I have fixed the path traversal security issue in config.ts, fixed the fee-bump calculation in client.ts, redacted the machine-specific data in stdout, and added a real client test around submitExtensionWithFeeBump as requested. Waiting for CodeRabbit review and CI to pass before merging. |
|
Here is a patch that resolves the CodeRabbit review issues (fee-bump base fee and sponsorSecret resolution) and fixes the test assertions: src/core/extension.ts | 14 ++++++++++++-- diff --git a/src/core/extension.ts b/src/core/extension.ts
diff --git a/src/rpc/client.ts b/src/rpc/client.ts
@@ -622,7 +622,7 @@ describe("Core Extension Logic", () => {
@@ -777,7 +777,7 @@ describe("Core Extension Logic", () => {
@@ -823,7 +823,7 @@ describe("Core Extension Logic", () => {
-- `` |
fixes #195