Feature/on chain settlement fen#671
Conversation
📝 WalkthroughWalkthroughThis PR consolidates game-state management by removing the separate Changes
Sequence DiagramsequenceDiagram
participant Frontend as Frontend App
participant Freighter as Freighter Wallet
participant SorobanRPC as Soroban RPC
participant Contract as Smart Contract
Frontend->>Frontend: Build transaction with contract.call(args)
Frontend->>SorobanRPC: simulateTransaction(tx)
SorobanRPC->>Contract: Execute simulation
Contract-->>SorobanRPC: Return transactionData
SorobanRPC-->>Frontend: Return simulation result
Frontend->>Freighter: signWithFreighter(xdr)
Freighter-->>Frontend: Return signed tx
Frontend->>SorobanRPC: sendTransaction(signedTx)
SorobanRPC->>Contract: Submit transaction
Contract-->>SorobanRPC: Return status
SorobanRPC-->>Frontend: Return result or error
alt Submission Error
Frontend->>Frontend: throw error
else Success
Frontend->>Frontend: Return response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@Abdullahi130 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/context/walletContext.tsx (2)
232-246:⚠️ Potential issue | 🟠 MajorFallback branch will also fail and masks the real error.
submitTransactionaccepts aTransaction/FeeBumpTransaction, not a raw XDR string — theas unknown as Transactioncast will serialize the wrong payload and the fallback will throw anyway. Also, re-entering a secondsubmitTransactionwipes out the originalerr(which likely contained Horizon's usefulextras.result_codes). Drop the fallback and surfaceerrdirectly.try { const txObj = TransactionBuilder.fromXDR(signedEnvelopeXDR, NETWORK_PASSPHRASE); const res = await server.submitTransaction(txObj); return res; } catch (err) { - // some horizon clients expect a TransactionEnvelope object; try submitting as-is - try { - // fallback: try submitting as-is (deprecated) - const res = await server.submitTransaction(signedEnvelopeXDR as unknown as Transaction); - return res; - } catch (e) { - console.error("submitTransaction failed", e); - throw e; - } + console.error("submitTransaction failed", err); + throw err; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/context/walletContext.tsx` around lines 232 - 246, The fallback branch that casts signedEnvelopeXDR to Transaction and re-calls server.submitTransaction should be removed because TransactionBuilder.fromXDR/submitTransaction expect proper Transaction/FeeBumpTransaction objects and the cast will fail and mask the original error; in the catch for TransactionBuilder.fromXDR(signedEnvelopeXDR, NETWORK_PASSPHRASE) simply log or re-throw the original err (preserving Horizon extras/result_codes) — i.e., delete the inner try/catch that attempts server.submitTransaction(signedEnvelopeXDR as unknown as Transaction) and replace it with a direct processLogger/console.error of err and throw err.
165-165:⚠️ Potential issue | 🔴 CriticalBug: useless ternary always calls the same function.
Both branches of the ternary are identical, so the guard does nothing and will throw if
freighter.getPublicKeyis undefined on a given variant (e.g. freighter-api v2 which exposesrequestAccess/getAddress).- const publicKey = await (freighter.getPublicKey ? freighter.getPublicKey() : freighter.getPublicKey()); + let publicKey: string | undefined; + if (typeof freighter.getAddress === "function") { + const res = await freighter.getAddress(); + publicKey = typeof res === "string" ? res : res?.address; + } else if (typeof freighter.getPublicKey === "function") { + publicKey = await freighter.getPublicKey(); + } else if (typeof freighter.requestAccess === "function") { + publicKey = await freighter.requestAccess(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/context/walletContext.tsx` at line 165, The ternary always calls freighter.getPublicKey and will throw when that method doesn't exist; update the publicKey acquisition to check for available methods and call the correct one: if freighter.getPublicKey exists await freighter.getPublicKey(), else if freighter.getAddress exists ensure access (await freighter.requestAccess() if present) then await freighter.getAddress(), otherwise handle the absent API (throw or return null). Replace the current line that sets publicKey with this conditional sequence using the freighter methods (getPublicKey, requestAccess, getAddress) and ensure the result is assigned to the publicKey variable.contracts/game_contract/src/lib.rs (3)
178-236:⚠️ Potential issue | 🟠 MajorAdd bounds on
initial_boardsize.
initial_board: Bytesis accepted verbatim into persistent instance storage with no length check. A caller can push a very large blob intoGame.board_fento inflate storage/CPU on every subsequent load of theGAMESmap (everyjoin_game,submit_move,payout,file_dispute, etc. deserializes the whole map). A standard FEN is ≤ ~90 bytes; cap it explicitly (e.g.<= 128) and reject oversized inputs up front.pub fn create_game( env: Env, player1: Address, wager_amount: i128, initial_board: Bytes, ) -> Result<u64, ContractError> { + if initial_board.len() > 128 { + return Err(ContractError::InvalidMove); + } let max_stake: i128 = env.storage().instance().get(&MAX_STAKE).unwrap_or(1_000);Apply the same bound to
new_boardinsubmit_move. As per the issue's "efficient resource utilization (Gas/CPU)" acceptance criterion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 178 - 236, Add an explicit size check (e.g. max 128 bytes) for board FEN data: in create_game, validate initial_board.len() <= 128 before constructing/storing the Game (and return a suitable ContractError such as BoardTooLarge) so oversized blobs are rejected up front instead of being persisted to Game.board_fen; do the same validation in submit_move for the new_board parameter before accepting/storing the move. Ensure you reference the create_game and submit_move functions and the Game.board_fen/new_board symbols when adding the checks and error return.
292-356:⚠️ Potential issue | 🔴 CriticalCritical trust bug: player-supplied
game_statuslets any player claim the entire pot.
submit_movetrusts the caller'sgame_statusargument and, when it's1, immediately marks the gameCompleted, setswinner = player, and callsprocess_payout(&env, &game, &player)— transferring both stakes (minus fee) to the submitter. Nothing on-chain verifies that the submitted move actually delivers checkmate, that the FEN is legal, or that the opponent agrees. A malicious player only needs to callsubmit_move(game_id, self, [anything_non_empty], any_bytes, 1)on their turn to drain the pot.game_status == 2has the symmetric bug for draws.This defeats the stated purpose of on-chain settlement (issue
#520). At minimum the contract must not accept the outcome as an input. Options in order of preference:
- Verify the terminal condition on-chain from
new_board(FEN parse + legal-move/mate/stalemate check).- Require dual attestation — both players must co-sign a settlement payload (e.g. a separate
settle_game(game_id, outcome, sig_p1, sig_p2)entrypoint), similar to the existingclaim_puzzle_rewardED25519 flow.- Require backend/oracle ED25519 signature over
(game_id, final_fen, outcome, nonce)and verify insubmit_moveusing the existingADMIN_KEYinfrastructure.Until one of the above is in place, revert
submit_moveto only recording the move + board and keep settlement on the existingclaim_draw/forfeit/ arbitrator paths.🛠️ Minimal interim patch — stop trusting the status flag
- pub fn submit_move( - env: Env, - game_id: u64, - player: Address, - move_data: Vec<u32>, - new_board: Bytes, - game_status: u32, - ) -> Result<(), ContractError> { + pub fn submit_move( + env: Env, + game_id: u64, + player: Address, + move_data: Vec<u32>, + new_board: Bytes, + ) -> Result<(), ContractError> { ... game.board_fen = new_board; - - // Auto settlement logic based on status flag - if game_status == 1 { - // Checkmate: submitting player wins - game.state = GameState::Completed; - game.winner = Some(player.clone()); - Self::process_payout(&env, &game, &player)?; - } else if game_status == 2 { - // Draw - game.state = GameState::Drawn; - Self::process_draw_payout(&env, &game)?; - } + // Settlement must go through an authenticated path + // (dual-signed `settle_game`, oracle-signed verification, or on-chain FEN+mate check). ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 292 - 356, submit_move currently trusts the caller-provided game_status and immediately finalizes and pays out via process_payout/process_draw_payout, allowing malicious players to steal the pot; remove that trust by deleting the auto-settlement branch in submit_move (the if game_status == 1 / else if game_status == 2 block), do not set game.state to Completed/Drawn, do not set game.winner, and do not call process_payout or process_draw_payout from submit_move — instead only record the move (ChessMove), update board_fen, last_move_at and current_turn as you already do and leave finalization to existing settlement flows (claim_draw, forfeit, arbitrator) or implement one of the recommended secure options (on-chain FEN verification, dual-attestation settle_game entrypoint, or oracle signature verification using ADMIN_KEY) in a separate entrypoint.
1905-1972:⚠️ Potential issue | 🟠 MajorTests exercise the new signature but do not cover auto-settlement.
The new
submit_move(..., new_board, game_status)has no test that passesgame_status = 1or2and verifies the winner/draw payout side-effect (or — once fixed per the critical comment above — that a bogus status is rejected). Given this is the entire point of the PR and the linked issue's acceptance criterion ("Unit tests cover standard and edge cases"), please add coverage for:
- checkmate auto-settlement transfers the pot to the submitter,
- draw auto-settlement returns stakes to both players,
- an unauthorized/unverified status value is rejected.
Want me to draft these tests once the settlement path is redesigned?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 1905 - 1972, Add unit tests exercising the new submit_move signature to cover auto-settlement and invalid statuses: create a game via create_game/join_game, fund players with StellarAssetClient::mint, initialize token with client.initialize_token and set_max_stake, then call submit_move with game_status = 1 and assert the submitter receives the pot (check token balances and game state via get_game), call submit_move with game_status = 2 and assert stakes are refunded to both players (check balances and game.moves/game.current_turn), and call try_submit_move with an invalid/unauthorized status and assert it returns an Err(Ok(...)) matching the contract error for invalid game status (mirror how NotYourTurn/InvalidMove are asserted already).
🧹 Nitpick comments (3)
contracts/game_contract/src/lib.rs (2)
292-338:game_statusis undocumented and uses magic numbers.Even after the settlement-trust issue above is addressed, the raw
u32with inline comments (1 = checkmate,2 = draw) is error-prone. Use a#[contracttype] enum MoveOutcome { Ongoing, Checkmate, Draw, ... }so the ABI is self-describing and invalid values are rejected at decode time.
1204-1206: Two#[cfg(test)] mods in the same file — confirm intentional split.Line 1205 declares
mod test;(externaltest.rs) and immediately below, line 1208 definesmod tests { ... }inline in this file. Both names compile, but it makes test discovery confusing (cargo testruns both, with overlapping setup helpers). Consider moving the inlinetestsblock intotest.rsas a submodule, or rename one (e.g.mod puzzle_tests) to clarify intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/game_contract/src/lib.rs` around lines 1204 - 1206, There are two cfg(test) modules declared — the external module `mod test;` and the inline `mod tests { ... }` — causing duplicate/overlapping test discovery; either move the inline `mod tests { ... }` contents into the external `test.rs` as a submodule under `test` or rename the inline module (e.g., `mod puzzle_tests`) and update any references to its helper functions, ensuring only one intentionally named test module provides shared setup helpers (`mod test` vs `mod tests`) to avoid confusion during `cargo test`.frontend/context/walletContext.tsx (1)
249-300: Input validation:contractId,functionName,argsare passed directly toContract.callwithout type coercion.
Contract.call(name, ...args)requiresScVals for args. If callers pass plain JS strings/numbers/Addressobjects (as increate_game(player1, wager_amount, initial_board)), the call will fail at build time or silently send incorrect data. Consider either (a) documenting that callers must supplyxdr.ScVals, or (b) accepting typed inputs here and converting vianativeToScValbeforecontract.call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/context/walletContext.tsx` around lines 249 - 300, invokeSorobanContract currently forwards contractId, functionName, and args directly into Contract.call which requires xdr.ScVal arguments; convert and validate inputs before calling: ensure contractId and functionName are strings (throw if not), map each entry of args through stellar-sdk's nativeToScVal (and handle Address/BigInt/number/boolean/null/array/object cases) to produce ScVal instances, then call contract.call(functionName, ...convertedArgs); import nativeToScVal where needed and add clear error messages for invalid arg types so callers don't send plain JS values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/WalletConnectModal.tsx`:
- Around line 113-116: In WalletConnectModal, the glowing aura div is inside
Dialog.Content with z-[-1], so it's rendered behind the dialog's semi-opaque
background and becomes invisible; move the aura out of Dialog.Content and place
it as a sibling (e.g., directly under the Dialog.Portal or the Dialog wrapper)
so it lives in the same portal but outside the dialog's painted background, keep
its positioning classes (absolute, -inset-1, rounded-2xl, bg-gradient-to-r,
blur-xl, opacity-70) and adjust stacking (give the aura a lower z-index than the
dialog container but not inside the dialog itself) so the glow is visible around
the modal edges; alternatively, if you prefer CSS, implement the aura as a
::before pseudo-element on the dialog wrapper element (e.g., the element
rendered by Dialog.Content) positioned outside the clipped rounded container so
the gradient shows through the edges.
In `@frontend/context/walletContext.tsx`:
- Line 281: The current call to TransactionBuilder.assembleTransaction(tx,
NETWORK_PASSPHRASE, simulatedTx) is incompatible with stellar-sdk v10.4.0;
replace the static call and wrong signature by importing SorobanRpc from
stellar-sdk and calling SorobanRpc.assembleTransaction(tx, simulatedTx) (then
.build())—remove NETWORK_PASSPHRASE and stop using
TransactionBuilder.assembleTransaction; update the import and the call site
where tx and simulatedTx are used.
- Around line 289-293: Replace the hardcoded string comparison and add polling
for a terminal transaction state: use the SDK enum
SorobanRpc.Api.SendTransactionStatus instead of the literal "ERROR" when
checking sendResponse.status (refer to sendTransaction and sendResponse), and
after receiving sendResponse.hash call
rpcServer.getTransaction(sendResponse.hash) in a short retry loop that handles
NOT_FOUND and PENDING states until the status becomes a terminal state (e.g.,
SUCCESS or FAILED per the SDK enum); if the final status is an error, throw with
the error details (use sendResponse.errorResult or the final getTransaction
result), otherwise return the final successful result to the caller.
---
Outside diff comments:
In `@contracts/game_contract/src/lib.rs`:
- Around line 178-236: Add an explicit size check (e.g. max 128 bytes) for board
FEN data: in create_game, validate initial_board.len() <= 128 before
constructing/storing the Game (and return a suitable ContractError such as
BoardTooLarge) so oversized blobs are rejected up front instead of being
persisted to Game.board_fen; do the same validation in submit_move for the
new_board parameter before accepting/storing the move. Ensure you reference the
create_game and submit_move functions and the Game.board_fen/new_board symbols
when adding the checks and error return.
- Around line 292-356: submit_move currently trusts the caller-provided
game_status and immediately finalizes and pays out via
process_payout/process_draw_payout, allowing malicious players to steal the pot;
remove that trust by deleting the auto-settlement branch in submit_move (the if
game_status == 1 / else if game_status == 2 block), do not set game.state to
Completed/Drawn, do not set game.winner, and do not call process_payout or
process_draw_payout from submit_move — instead only record the move (ChessMove),
update board_fen, last_move_at and current_turn as you already do and leave
finalization to existing settlement flows (claim_draw, forfeit, arbitrator) or
implement one of the recommended secure options (on-chain FEN verification,
dual-attestation settle_game entrypoint, or oracle signature verification using
ADMIN_KEY) in a separate entrypoint.
- Around line 1905-1972: Add unit tests exercising the new submit_move signature
to cover auto-settlement and invalid statuses: create a game via
create_game/join_game, fund players with StellarAssetClient::mint, initialize
token with client.initialize_token and set_max_stake, then call submit_move with
game_status = 1 and assert the submitter receives the pot (check token balances
and game state via get_game), call submit_move with game_status = 2 and assert
stakes are refunded to both players (check balances and
game.moves/game.current_turn), and call try_submit_move with an
invalid/unauthorized status and assert it returns an Err(Ok(...)) matching the
contract error for invalid game status (mirror how NotYourTurn/InvalidMove are
asserted already).
In `@frontend/context/walletContext.tsx`:
- Around line 232-246: The fallback branch that casts signedEnvelopeXDR to
Transaction and re-calls server.submitTransaction should be removed because
TransactionBuilder.fromXDR/submitTransaction expect proper
Transaction/FeeBumpTransaction objects and the cast will fail and mask the
original error; in the catch for TransactionBuilder.fromXDR(signedEnvelopeXDR,
NETWORK_PASSPHRASE) simply log or re-throw the original err (preserving Horizon
extras/result_codes) — i.e., delete the inner try/catch that attempts
server.submitTransaction(signedEnvelopeXDR as unknown as Transaction) and
replace it with a direct processLogger/console.error of err and throw err.
- Line 165: The ternary always calls freighter.getPublicKey and will throw when
that method doesn't exist; update the publicKey acquisition to check for
available methods and call the correct one: if freighter.getPublicKey exists
await freighter.getPublicKey(), else if freighter.getAddress exists ensure
access (await freighter.requestAccess() if present) then await
freighter.getAddress(), otherwise handle the absent API (throw or return null).
Replace the current line that sets publicKey with this conditional sequence
using the freighter methods (getPublicKey, requestAccess, getAddress) and ensure
the result is assigned to the publicKey variable.
---
Nitpick comments:
In `@contracts/game_contract/src/lib.rs`:
- Around line 1204-1206: There are two cfg(test) modules declared — the external
module `mod test;` and the inline `mod tests { ... }` — causing
duplicate/overlapping test discovery; either move the inline `mod tests { ... }`
contents into the external `test.rs` as a submodule under `test` or rename the
inline module (e.g., `mod puzzle_tests`) and update any references to its helper
functions, ensuring only one intentionally named test module provides shared
setup helpers (`mod test` vs `mod tests`) to avoid confusion during `cargo
test`.
In `@frontend/context/walletContext.tsx`:
- Around line 249-300: invokeSorobanContract currently forwards contractId,
functionName, and args directly into Contract.call which requires xdr.ScVal
arguments; convert and validate inputs before calling: ensure contractId and
functionName are strings (throw if not), map each entry of args through
stellar-sdk's nativeToScVal (and handle
Address/BigInt/number/boolean/null/array/object cases) to produce ScVal
instances, then call contract.call(functionName, ...convertedArgs); import
nativeToScVal where needed and add clear error messages for invalid arg types so
callers don't send plain JS values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e525ee8a-bb6b-4898-a3b3-64fcaf2ef4e3
📒 Files selected for processing (6)
contracts/game_contract/src/game_state.rscontracts/game_contract/src/lib.rscontracts/game_contract/src/test.rsfrontend/components/WalletConnectModal.tsxfrontend/components/Web3StatusBar.tsxfrontend/context/walletContext.tsx
💤 Files with no reviewable changes (1)
- contracts/game_contract/src/game_state.rs
| <Dialog.Content className="fixed left-1/2 top-1/2 z-50 w-full max-w-md -translate-x-1/2 -translate-y-1/2 rounded-2xl border border-gray-600/50 bg-gray-900/80 backdrop-blur-2xl p-0 shadow-[0_0_50px_-12px_rgba(20,184,166,0.3)] animate-modal-in focus:outline-none"> | ||
| {/* Glowing Aura Background */} | ||
| <div className="absolute -inset-1 rounded-2xl bg-gradient-to-r from-teal-500/20 to-blue-600/20 blur-xl z-[-1] opacity-70 pointer-events-none"></div> | ||
|
|
There was a problem hiding this comment.
Glowing aura will likely be invisible behind the modal's own background.
The aura is absolute z-[-1] inside a Dialog.Content that itself has bg-gray-900/80 backdrop-blur-2xl. Negative z within the same stacking context puts the aura behind the parent's painted background, so the blurred gradient won't show. Options: move the aura outside Dialog.Content (as a sibling in the portal) or use a ::before pseudo-element with the aura positioned outside the clipped rounded container so it glows through the edges.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/WalletConnectModal.tsx` around lines 113 - 116, In
WalletConnectModal, the glowing aura div is inside Dialog.Content with z-[-1],
so it's rendered behind the dialog's semi-opaque background and becomes
invisible; move the aura out of Dialog.Content and place it as a sibling (e.g.,
directly under the Dialog.Portal or the Dialog wrapper) so it lives in the same
portal but outside the dialog's painted background, keep its positioning classes
(absolute, -inset-1, rounded-2xl, bg-gradient-to-r, blur-xl, opacity-70) and
adjust stacking (give the aura a lower z-index than the dialog container but not
inside the dialog itself) so the glow is visible around the modal edges;
alternatively, if you prefer CSS, implement the aura as a ::before
pseudo-element on the dialog wrapper element (e.g., the element rendered by
Dialog.Content) positioned outside the clipped rounded container so the gradient
shows through the edges.
| throw new Error("Simulation failed: missing transactionData"); | ||
| } | ||
|
|
||
| tx = TransactionBuilder.assembleTransaction(tx, NETWORK_PASSPHRASE, simulatedTx as any).build(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
stellar-sdk SorobanRpc assembleTransaction signature arguments
💡 Result:
In the @stellar/stellar-sdk JavaScript library, assembleTransaction is a static function exported from the RPC module (lib/rpc/transaction.js). It combines a raw Soroban transaction with simulation results from the Soroban RPC simulateTransaction method to produce a TransactionBuilder with applied authorization data, resources (fee, footprint), and Soroban transaction data. Signature: assembleTransaction(raw, simulation) → TransactionBuilder Arguments: - raw: Transaction or FeeBumpTransaction - the initial transaction without simulation applied (must contain exactly one invokeHostFunction, extendFootprintTtl, or restoreFootprint operation). - simulation: Api.SimulateTransactionResponse or Api.RawSimulateTransactionResponse - the Soroban RPC simulation result. The function: - Handles FeeBumpTransactions by recursing to the inner transaction. - Validates the transaction is Soroban-compatible and simulation succeeded. - Adjusts fees to sum classic fee and simulation minResourceFee (avoiding double-counting if raw already has SorobanData). - Clones the raw transaction via TransactionBuilder.cloneFrom, applying simulation SorobanData. - For invokeHostFunction ops, replaces auth with simulation auth (unless raw already has auth, then keeps existing). This is typically used internally by higher-level methods like Server.prepareTransaction (which simulates then assembles) or AssembledTransaction flows, but can be called directly for advanced control. Example usage pattern (from SDK patterns): const simulation = await server.simulateTransaction(rawTx); const assembledBuilder = rpc.assembleTransaction(rawTx, simulation); // Then sign: assembledBuilder.sign(keypair); const tx = assembledBuilder.build; Note: Soroban RPC server itself has no assembleTransaction method; standard RPC methods are simulateTransaction, sendTransaction, etc. The function is SDK-side. Official source: @stellar/stellar-sdk documentation.
Citations:
- 1: https://stellar.github.io/js-stellar-sdk/lib_rpc_transaction.js.html
- 2: http://stellar.github.io/js-stellar-sdk/module-rpc.html
🏁 Script executed:
# Check the imports and package information
cd frontend/context && head -50 walletContext.tsx | grep -E "^import|^from"Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Find package.json to check stellar-sdk version
fd package.json | head -5 | xargs grep -l "stellar-sdk" 2>/dev/nullRepository: NOVUS-X/XLMate
Length of output: 79
🏁 Script executed:
# Search for other uses of assembleTransaction in the codebase
rg "assembleTransaction" -A 2 -B 2Repository: NOVUS-X/XLMate
Length of output: 380
🏁 Script executed:
head -100 frontend/context/walletContext.tsx | cat -nRepository: NOVUS-X/XLMate
Length of output: 3973
🏁 Script executed:
cat frontend/package.json | grep -A 2 -B 2 "stellar"Repository: NOVUS-X/XLMate
Length of output: 392
🏁 Script executed:
sed -n '260,290p' frontend/context/walletContext.tsx | cat -nRepository: NOVUS-X/XLMate
Length of output: 1448
🏁 Script executed:
rg "import.*TransactionBuilder" frontend/context/walletContext.tsxRepository: NOVUS-X/XLMate
Length of output: 143
🏁 Script executed:
rg "import.*SorobanRpc\|import.*rpc" frontend/context/walletContext.tsxRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
head -50 frontend/context/walletContext.tsx | cat -n | grep -v "^[[:space:]]*[0-9]\+[[:space:]]*\/\/"Repository: NOVUS-X/XLMate
Length of output: 97
🏁 Script executed:
rg "rpcServer\s*=" frontend/context/walletContext.tsx | head -5Repository: NOVUS-X/XLMate
Length of output: 127
🏁 Script executed:
rg "import.*Server" frontend/context/walletContext.tsxRepository: NOVUS-X/XLMate
Length of output: 143
assembleTransaction call is incompatible with stellar-sdk 10.4.0 API.
The method signature is assembleTransaction(raw, simulation) with 2 arguments only. The NETWORK_PASSPHRASE parameter is incorrect and the method is not available as a static on TransactionBuilder. Import SorobanRpc from stellar-sdk and call:
tx = SorobanRpc.assembleTransaction(tx, simulatedTx).build();
This will fail at runtime with the current implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/context/walletContext.tsx` at line 281, The current call to
TransactionBuilder.assembleTransaction(tx, NETWORK_PASSPHRASE, simulatedTx) is
incompatible with stellar-sdk v10.4.0; replace the static call and wrong
signature by importing SorobanRpc from stellar-sdk and calling
SorobanRpc.assembleTransaction(tx, simulatedTx) (then .build())—remove
NETWORK_PASSPHRASE and stop using TransactionBuilder.assembleTransaction; update
the import and the call site where tx and simulatedTx are used.
| const sendResponse = await rpcServer.sendTransaction(signedTx); | ||
|
|
||
| if (sendResponse.status === "ERROR") { | ||
| throw new Error(`Transaction failed: ${JSON.stringify(sendResponse.errorResult)}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
stellar-sdk SorobanRpc sendTransaction getTransaction poll status PENDING SUCCESS
💡 Result:
To use Stellar SDK's SorobanRpc to send a transaction, poll its status until PENDING, SUCCESS, or other final states: 1. Build, prepare, and sign your transaction using TransactionBuilder, server.prepareTransaction(tx), and tx.sign(keypair). 2. Call server.sendTransaction(signedTx) which returns a SendTransactionResponse with status typically "PENDING" and transaction hash. 3. Poll using server.getTransaction(hash) repeatedly until status changes from "NOT_FOUND" or "PENDING". Key statuses: - PENDING: Enqueued but not processed. - SUCCESS: Applied successfully (includes ledger, resultXdr, etc.). - FAILED: Failed (includes error details). - NOT_FOUND: Not yet visible or expired. - ERROR: Immediate rejection. Example polling loop (from SDK guides and examples): async function submitAndWait(server, tx) { const sendResult = await server.sendTransaction(tx); if (sendResult.status !== 'PENDING') { throw new Error(Send failed: ${sendResult.status}); } let txResult; while (true) { txResult = await server.getTransaction(sendResult.hash); if (txResult.status !== 'NOT_FOUND') { break; } await new Promise(resolve => setTimeout(resolve, 1000)); // Wait 1s } if (txResult.status === 'SUCCESS') { return txResult; } else { throw new Error(Transaction ${txResult.status}: ${txResult.errorResultXdr || ''}); } } Newer @stellar/stellar-sdk provides server.pollTransaction(hash, {sleepStrategy: => 500, attempts: 5}) for built-in polling. Status enum: SorobanRpc.Api.GetTransactionStatus.PENDING, .SUCCESS, .FAILED, .NOT_FOUND, .ERROR.
Citations:
- 1: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/sendTransaction
- 2: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getTransaction
- 3: https://developers.stellar.org/docs/build/guides/transactions/submit-transaction-wait-js
- 4: Can't submit transaction using freighter api stellar/freighter#1071
- 5: https://stellar.github.io/js-soroban-client/Server.html
- 6: http://stellar.github.io/js-stellar-sdk/module-rpc.Server.html
🏁 Script executed:
cat -n frontend/context/walletContext.tsx | sed -n '270,310p'Repository: NOVUS-X/XLMate
Length of output: 1634
🏁 Script executed:
head -50 frontend/context/walletContext.tsxRepository: NOVUS-X/XLMate
Length of output: 1629
🏁 Script executed:
rg "^import|^from" frontend/context/walletContext.tsx | head -30Repository: NOVUS-X/XLMate
Length of output: 265
🏁 Script executed:
rg "rpcServer" frontend/context/walletContext.tsx | head -20Repository: NOVUS-X/XLMate
Length of output: 320
🏁 Script executed:
rg -B 5 "const invokeSorobanContract" frontend/context/walletContext.tsx | head -20Repository: NOVUS-X/XLMate
Length of output: 198
🏁 Script executed:
rg "invokeSorobanContract" frontend/ -A 2 -B 1 | head -40Repository: NOVUS-X/XLMate
Length of output: 1134
🏁 Script executed:
rg "SendTransactionResponse|GetTransactionStatus|SendTransactionStatus" frontend/ -A 3 | head -40Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
rg "stellar-sdk" package.jsonRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
fd package.json | head -5Repository: NOVUS-X/XLMate
Length of output: 92
🏁 Script executed:
cat frontend/package.json | grep -A 2 -B 2 "stellar-sdk"Repository: NOVUS-X/XLMate
Length of output: 213
🏁 Script executed:
cat package.json | grep -A 2 -B 2 "stellar-sdk"Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
web_searchRepository: NOVUS-X/XLMate
Length of output: 108
🌐 Web query:
stellar-sdk 10.4.0 SendTransactionResponse status enum ERROR PENDING
💡 Result:
In stellar-sdk version 10.4.0, the SendTransactionResponse from server.sendTransaction (Soroban RPC method) has a 'status' field with possible enum values: PENDING, DUPLICATE, TRY_AGAIN_LATER, and ERROR. PENDING indicates successful enqueuing (poll getTransaction for completion). ERROR includes errorResultXdr and possibly diagnosticEventsXdr with rejection details. DUPLICATE and TRY_AGAIN_LATER handle retries/network issues. These match official Stellar Soroban RPC docs and SDK source code (parseRawSendTransaction). Version 10.4.0 (confirmed in CHANGELOG) supports this via js-stellar-sdk PRs adding RPC response parsing.
Citations:
- 1: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/sendTransaction
- 2: https://developers.stellar.org/docs/data/rpc/api-reference/methods/sendTransaction
- 3: https://github.com/stellar/js-stellar-sdk/blob/master/CHANGELOG.md
- 4: https://github.com/stellar/js-stellar-sdk
- 5: https://github.com/stellar/js-stellar-sdk/blob/2f52d0e7/src/rpc/server.ts
- 6: Add support for new
sendTransactionresponse field stellar/js-stellar-sdk#905 - 7: Add support for new fields in Soroban RPC's
sendTransactionendpoint stellar/js-stellar-sdk#910
Use the SDK enum for status comparison and poll for terminal transaction state before returning.
The code compares against the bare string "ERROR" instead of using SorobanRpc.Api.SendTransactionStatus.ERROR from the SDK. More critically, sendTransaction() returns PENDING when the transaction is enqueued but not yet applied—the function returns immediately (line 295) without polling getTransaction(hash) until reaching a terminal status (SUCCESS, FAILED, etc.). This means callers of invokeSorobanContract will incorrectly believe the transaction succeeded when it's only enqueued. You must poll getTransaction(sendResponse.hash) until the status changes from PENDING or NOT_FOUND before returning to the caller.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/context/walletContext.tsx` around lines 289 - 293, Replace the
hardcoded string comparison and add polling for a terminal transaction state:
use the SDK enum SorobanRpc.Api.SendTransactionStatus instead of the literal
"ERROR" when checking sendResponse.status (refer to sendTransaction and
sendResponse), and after receiving sendResponse.hash call
rpcServer.getTransaction(sendResponse.hash) in a short retry loop that handles
NOT_FOUND and PENDING states until the status becomes a terminal state (e.g.,
SUCCESS or FAILED per the SDK enum); if the final status is an error, throw with
the error details (use sendResponse.errorResult or the final getTransaction
result), otherwise return the final successful result to the caller.
1 participant
I have successfully implemented Contract: On-chain Puzzle Reward Verification,
I look forward to working with you next time,
Thank you.
closes #520
Summary by CodeRabbit
New Features
Style
Improvements