Open
Conversation
WalkthroughAdds Jupiter Protocol support and a Solana swap tool: introduces a Jupiter client, wires a Solana RPC-backed Solana client, registers a new build-solana-swap tool, updates configuration for JUPITER_API_URL, and adapts registrations/tests to pass rpc.Client and jupiter.Client instances. Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
internal/solana/client.go (1)
255-268: Verify ATA createIdempotent instruction discriminator.The instruction data
[]byte{1}forcreateIdempotentshould be verified. The comment indicates this is the correct discriminator, but a reference to the ATA program specification would help maintainability.📝 Add reference comment
func buildCreateATAInstruction(payer, owner, mint, ataAddress, tokenProgram solana.PublicKey) solana.Instruction { return solana.NewInstruction( solana.SPLAssociatedTokenAccountProgramID, []*solana.AccountMeta{ {PublicKey: payer, IsSigner: true, IsWritable: true}, {PublicKey: ataAddress, IsSigner: false, IsWritable: true}, {PublicKey: owner, IsSigner: false, IsWritable: false}, {PublicKey: mint, IsSigner: false, IsWritable: false}, {PublicKey: solana.SystemProgramID, IsSigner: false, IsWritable: false}, {PublicKey: tokenProgram, IsSigner: false, IsWritable: false}, }, - []byte{1}, // createIdempotent: no-op if ATA already exists + // Instruction discriminator 1 = CreateIdempotent (no-op if ATA already exists) + // See: https://github.com/solana-labs/solana-program-library/blob/master/associated-token-account/program/src/instruction.rs + []byte{1}, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/solana/client.go` around lines 255 - 268, The instruction data literal []byte{1} in buildCreateATAInstruction should be verified and documented as the SPL Associated Token Account "createIdempotent" discriminator; confirm the value matches the ATA program spec/source, replace the magic literal with a named constant (e.g. createIdempotentInstruction = []byte{1}) and add a comment linking to the SPL Associated Token Account program docs/source that defines the discriminator so future readers can verify it.internal/tools/build_solana_tx.go (1)
57-70: Consider validating from != to.The tool allows building transactions where sender and recipient are the same address. While technically valid, self-transfers waste transaction fees. Consider adding a validation check.
🛡️ Optional: Add self-transfer validation
toPubkey, err := solanaclient.ParsePublicKey(toStr) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("invalid to address: %v", err)), nil } + if fromPubkey == toPubkey { + return mcp.NewToolResultError("from and to addresses cannot be the same"), nil + } + txBytes, err := solClient.BuildNativeTransfer(ctx, fromPubkey, toPubkey, amount.Uint64())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tools/build_solana_tx.go` around lines 57 - 70, After parsing fromAddr and toStr into fromPubkey and toPubkey (after the calls to solanaclient.ParsePublicKey), add a validation that the sender and recipient are not the same; if they are equal return mcp.NewToolResultError with a clear message like "from and to addresses must differ" (compare either the parsed public keys via their equality method or compare their byte/string representations). Place this check immediately after toPubkey is obtained so the function rejects self-transfers early and avoids building a no-op transaction.internal/tools/build_solana_swap.go (1)
64-67: Consider adding upper bound validation for amount.Unlike
build_spl_transfer_tx.gowhich validates!amount.IsUint64()to prevent overflow, this handler only checksamount.Sign() <= 0. While Jupiter's API accepts string amounts, extremely large values could cause unexpected behavior. Consider adding a reasonable upper bound or documenting the expected range.Optional: Add consistency with other tools
amount, ok := new(big.Int).SetString(amountStr, 10) - if !ok || amount.Sign() <= 0 { + if !ok || amount.Sign() <= 0 || !amount.IsUint64() { return mcp.NewToolResultError(fmt.Sprintf("invalid amount: %s", amountStr)), nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tools/build_solana_swap.go` around lines 64 - 67, The amount parsing only checks for non-positive values; add an upper-bound check like in build_spl_transfer_tx.go to prevent overflow/huge inputs: after parsing with new(big.Int).SetString and the existing amount.Sign() <= 0 check, verify the value fits an expected range (for example ensure amount.IsUint64() or compare against a defined maxBigInt) and return the same mcp.NewToolResultError(fmt.Sprintf("invalid amount: %s", amountStr)) on failure; update the validation around the amount variable and keep the existing error path consistent.internal/tools/tools.go (1)
21-21: Function signature is growing - consider a config/dependencies struct in the future.
RegisterAllnow has 10 parameters. While acceptable for now, if more clients are added, consider grouping related dependencies (e.g.,SolanaDeps{SolClient, JupClient}or a generalDepsstruct) to improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tools/tools.go` at line 21, RegisterAll currently takes many parameters (server.MCPServer, vault.Store, evmclient.Pool, coingecko.Client, blockchair.Client, swap.Service, btcsdk.Builder, thorchain.Client, solanaclient.Client, jupiter.Client); refactor by introducing one or more dependency structs (e.g., Deps or grouped structs like SolanaDeps { SolClient, JupClient }) and replace the long parameter list with those structs in the RegisterAll signature and its callers; update all call sites to construct and pass the new struct(s) and adjust any internal references inside RegisterAll to use the struct fields (retain types and names such as RegisterAll and the specific client/service field names to make locating changes straightforward).internal/jupiter/client.go (2)
240-249: Response body is included in error messages for non-200 responses.The error message at line 248 includes the full response body, which could potentially expose sensitive information in logs. For a swap API this is likely fine (route errors, rate limits), but be mindful if the API ever returns user-specific data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client.go` around lines 240 - 249, The error currently includes the full response body (read into variable body from io.ReadAll(resp.Body)) when resp.StatusCode != http.StatusOK, which can leak sensitive data; change the error handling in the response-processing block to avoid logging the entire body—either omit the body from the fmt.Errorf message or include a safely truncated/sanitized snippet (e.g., first 200 bytes) and indicate when it was truncated, and ensure you still close resp.Body; update the fmt.Errorf call that references resp.StatusCode and string(body) accordingly (replace string(body) with a safe preview or a generic message).
57-62: Consider validating that input and output mints differ.Both
inputMintandoutputMintdefault tosolana.SolMint.String()when empty. If both are left empty by the caller, this would attempt a SOL-to-SOL swap, which would fail at the Jupiter API level. Consider adding validation to fail fast with a clear error message.Proposed validation
if inputMint == "" { inputMint = solana.SolMint.String() } if outputMint == "" { outputMint = solana.SolMint.String() } + if inputMint == outputMint { + return nil, fmt.Errorf("input and output mints cannot be the same") + } if slippageBps <= 0 { slippageBps = DefaultSlippageBps }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client.go` around lines 57 - 62, After applying the defaulting logic for inputMint and outputMint (where both fallback to solana.SolMint.String()), add a validation step that checks if inputMint == outputMint and, if so, return an explicit error (e.g., "input and output mints must differ; SOL-to-SOL swap is invalid") so the function fails fast with a clear message; place this check immediately after the defaulting block that sets inputMint and outputMint to solana.SolMint.String() to ensure callers that omit both mints get the validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/mcp-server/main.go`:
- Around line 57-62: Add URL validation for cfg.SolanaRPCURL and
cfg.JupiterAPIURL inside config.Load(): parse each value (e.g., with
net/url.Parse) and ensure they have a valid scheme (http or https) and a
non-empty host; if invalid return an error from config.Load() so startup fails
fast. Update any call sites that assume config.Load() always succeeds to handle
the returned error (the code that uses cfg.SolanaRPCURL and
cfg.JupiterAPIURL—e.g., where solanaRPC := rpc.New(cfg.SolanaRPCURL) and
jupClient := jupiter.NewClient(cfg.JupiterAPIURL, solanaRPC) are invoked—will
then only run with validated URLs).
In `@internal/tools/tools.go`:
- Around line 41-45: The created RPC clients (solanaRPC, jupClient via rpc.New)
may be nil and will cause panics when tools call methods (e.g.,
handleGetSOLBalance -> get_sol_balance.go). After constructing the clients in
main (where rpc.New is called) validate their return values and either exit/log
and fail fast or skip/register only the tools that require a valid client;
specifically check solanaRPC and jupClient before calling
s.AddTool(newGetSOLBalanceTool(), handleGetSOLBalance(store, solClient)),
newBuildSolanaSwapTool()/handleBuildSolanaSwap, and the other handlers, or
alternatively make the handler constructors (handleGetSOLBalance,
handleBuildSolanaTx, handleBuildSPLTransferTx, handleBuildSolanaSwap) return an
error or a nil-safe handler and propagate that so you never call methods on a
nil RPC client.
---
Nitpick comments:
In `@internal/jupiter/client.go`:
- Around line 240-249: The error currently includes the full response body (read
into variable body from io.ReadAll(resp.Body)) when resp.StatusCode !=
http.StatusOK, which can leak sensitive data; change the error handling in the
response-processing block to avoid logging the entire body—either omit the body
from the fmt.Errorf message or include a safely truncated/sanitized snippet
(e.g., first 200 bytes) and indicate when it was truncated, and ensure you still
close resp.Body; update the fmt.Errorf call that references resp.StatusCode and
string(body) accordingly (replace string(body) with a safe preview or a generic
message).
- Around line 57-62: After applying the defaulting logic for inputMint and
outputMint (where both fallback to solana.SolMint.String()), add a validation
step that checks if inputMint == outputMint and, if so, return an explicit error
(e.g., "input and output mints must differ; SOL-to-SOL swap is invalid") so the
function fails fast with a clear message; place this check immediately after the
defaulting block that sets inputMint and outputMint to solana.SolMint.String()
to ensure callers that omit both mints get the validation.
In `@internal/solana/client.go`:
- Around line 255-268: The instruction data literal []byte{1} in
buildCreateATAInstruction should be verified and documented as the SPL
Associated Token Account "createIdempotent" discriminator; confirm the value
matches the ATA program spec/source, replace the magic literal with a named
constant (e.g. createIdempotentInstruction = []byte{1}) and add a comment
linking to the SPL Associated Token Account program docs/source that defines the
discriminator so future readers can verify it.
In `@internal/tools/build_solana_swap.go`:
- Around line 64-67: The amount parsing only checks for non-positive values; add
an upper-bound check like in build_spl_transfer_tx.go to prevent overflow/huge
inputs: after parsing with new(big.Int).SetString and the existing amount.Sign()
<= 0 check, verify the value fits an expected range (for example ensure
amount.IsUint64() or compare against a defined maxBigInt) and return the same
mcp.NewToolResultError(fmt.Sprintf("invalid amount: %s", amountStr)) on failure;
update the validation around the amount variable and keep the existing error
path consistent.
In `@internal/tools/build_solana_tx.go`:
- Around line 57-70: After parsing fromAddr and toStr into fromPubkey and
toPubkey (after the calls to solanaclient.ParsePublicKey), add a validation that
the sender and recipient are not the same; if they are equal return
mcp.NewToolResultError with a clear message like "from and to addresses must
differ" (compare either the parsed public keys via their equality method or
compare their byte/string representations). Place this check immediately after
toPubkey is obtained so the function rejects self-transfers early and avoids
building a no-op transaction.
In `@internal/tools/tools.go`:
- Line 21: RegisterAll currently takes many parameters (server.MCPServer,
vault.Store, evmclient.Pool, coingecko.Client, blockchair.Client, swap.Service,
btcsdk.Builder, thorchain.Client, solanaclient.Client, jupiter.Client); refactor
by introducing one or more dependency structs (e.g., Deps or grouped structs
like SolanaDeps { SolClient, JupClient }) and replace the long parameter list
with those structs in the RegisterAll signature and its callers; update all call
sites to construct and pass the new struct(s) and adjust any internal references
inside RegisterAll to use the struct fields (retain types and names such as
RegisterAll and the specific client/service field names to make locating changes
straightforward).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
CLAUDE.mdcmd/mcp-server/main.gogo.modinternal/config/config.gointernal/jupiter/client.gointernal/jupiter/client_test.gointernal/resolve/resolve.gointernal/solana/client.gointernal/solana/client_test.gointernal/tools/build_solana_swap.gointernal/tools/build_solana_tx.gointernal/tools/build_spl_transfer_tx.gointernal/tools/get_sol_balance.gointernal/tools/get_spl_token_balance.gointernal/tools/solana_tx_test.gointernal/tools/tools.go
4others
force-pushed
the
544-solana-swap
branch
from
9b3805d to
20400b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/jupiter/client_test.go (1)
94-238: Add direct tests forBuildSwapTransactionbranch behaviorCurrent tests are solid for
GetQuote/GetSwapInstructions, but they do not exerciseBuildSwapTransactionpaths like ALT rejection and invalid amount handling. A focused test here would lock in expected behavior and prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client_test.go` around lines 94 - 238, Add unit tests in internal/jupiter/client_test.go that directly exercise Client.BuildSwapTransaction: create one test (e.g., TestBuildSwapTransaction_ALTRejected) which sets up an httptest.Server returning a SwapInstructionsResponse crafted to trigger the ALT-rejection branch (simulate the condition your code checks in BuildSwapTransaction, e.g., a SwapInstruction payload or ProgramId that causes the ALT path) then call NewClient(...).BuildSwapTransaction(ctx, quote, payerPubkey) and assert it returns the expected non-nil error for ALT rejection; add another test (e.g., TestBuildSwapTransaction_InvalidAmount) that sends a QuoteResponse/SwapInstructionsResponse with mismatched or out-of-range amounts and assert BuildSwapTransaction returns the invalid-amount error; reference BuildSwapTransaction, SwapInstructionsResponse, SwapInstruction, QuoteResponse and NewClient to locate the code and ensure assertions check for non-nil error and appropriate error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/jupiter/client.go`:
- Around line 160-171: In GetQuote, guard against a nil amount before calling
amount.String(): check if amount == nil at the start of GetQuote and return a
descriptive error (e.g., fmt.Errorf("amount is nil")) instead of calling
amount.String() to avoid a panic; update the function to validate amount (and
optionally slippageBps if needed) and return early with an error so callers
receive a controlled failure rather than a process-level panic.
- Around line 194-200: The swapRequest sent in GetSwapInstructions (the reqBody
variable) must include AsLegacyTransaction: true to avoid Jupiter returning
AddressLookupTableAddresses that our client rejects; update the reqBody
construction (swapRequest) to set AsLegacyTransaction: true so the API returns
legacy transactions and prevents hard failures when AddressLookupTableAddresses
are present.
---
Nitpick comments:
In `@internal/jupiter/client_test.go`:
- Around line 94-238: Add unit tests in internal/jupiter/client_test.go that
directly exercise Client.BuildSwapTransaction: create one test (e.g.,
TestBuildSwapTransaction_ALTRejected) which sets up an httptest.Server returning
a SwapInstructionsResponse crafted to trigger the ALT-rejection branch (simulate
the condition your code checks in BuildSwapTransaction, e.g., a SwapInstruction
payload or ProgramId that causes the ALT path) then call
NewClient(...).BuildSwapTransaction(ctx, quote, payerPubkey) and assert it
returns the expected non-nil error for ALT rejection; add another test (e.g.,
TestBuildSwapTransaction_InvalidAmount) that sends a
QuoteResponse/SwapInstructionsResponse with mismatched or out-of-range amounts
and assert BuildSwapTransaction returns the invalid-amount error; reference
BuildSwapTransaction, SwapInstructionsResponse, SwapInstruction, QuoteResponse
and NewClient to locate the code and ensure assertions check for non-nil error
and appropriate error messages.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CLAUDE.mdcmd/mcp-server/main.gointernal/config/config.gointernal/jupiter/client.gointernal/jupiter/client_test.gointernal/solana/client.gointernal/solana/client_test.gointernal/tools/build_solana_swap.gointernal/tools/solana_tx_test.gointernal/tools/tools.go
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/mcp-server/main.go
- internal/tools/build_solana_swap.go
- internal/config/config.go
- CLAUDE.md
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
internal/jupiter/client.go (2)
160-171:⚠️ Potential issue | 🟠 MajorGuard
amountbefore callingamount.String().Line 170 can panic on nil input. Also reject non-positive values early so callers get a controlled error.
🛠️ Suggested fix
func (c *Client) GetQuote( ctx context.Context, inputMint, outputMint string, amount *big.Int, slippageBps int, ) (QuoteResponse, error) { + if amount == nil || amount.Sign() <= 0 { + return QuoteResponse{}, fmt.Errorf("amount must be a positive integer") + } + params := url.Values{} params.Set("swapMode", "ExactIn") params.Set("inputMint", inputMint)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client.go` around lines 160 - 171, In Client.GetQuote, guard against a nil or non-positive amount before calling amount.String(): validate that the amount parameter is not nil and that amount.Cmp(big.NewInt(0)) > 0, and return a descriptive error if the check fails; only after that proceed to build params (where amount.String() is used) so callers get a controlled error instead of a panic from amount.String().
194-200:⚠️ Potential issue | 🟠 MajorRequest legacy swap instructions to avoid predictable ALT rejection.
The client rejects
AddressLookupTableAddresseslater, but request payload does not ask Jupiter for legacy transaction instructions. This creates avoidable failures.🛠️ Suggested fix
reqBody := swapRequest{ UserPublicKey: userPublicKey, QuoteResponse: quote, WrapAndUnwrapSol: true, UseSharedAccounts: true, + AsLegacyTransaction: true, DynamicComputeUnitLimit: true, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client.go` around lines 194 - 200, The swapRequest payload currently created in the swapRequest construction (symbol: swapRequest and fields WrapAndUnwrapSol, UseSharedAccounts, DynamicComputeUnitLimit) does not request legacy transaction instructions; update the payload to explicitly request legacy swap instructions by adding the Jupiter legacy flag (e.g., set the appropriate field such as UseLegacyTransaction / UseLegacyInstructions / UseLegacyTransactions to true) when building the swapRequest so Jupiter returns legacy txn instructions and avoids ALT-related rejections; modify the swapRequest construction site where UserPublicKey and QuoteResponse are set to include that legacy flag.
🧹 Nitpick comments (2)
internal/jupiter/client_test.go (1)
180-190: AssertasLegacyTransactionin swap-instructions payload.This test should also verify
body.AsLegacyTransaction == trueto prevent regressions where the client asks for instructions that later get rejected due to lookup tables.✅ Suggested test hardening
if !body.DynamicComputeUnitLimit { t.Error("expected dynamicComputeUnitLimit=true") } + if !body.AsLegacyTransaction { + t.Error("expected asLegacyTransaction=true") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jupiter/client_test.go` around lines 180 - 190, The test decoding the swap request must also assert that the AsLegacyTransaction flag is set to true to catch regressions; after decoding into the local variable `body` (type `swapRequest`) and the existing assertions for `WrapAndUnwrapSol` and `DynamicComputeUnitLimit`, add an assertion that `body.AsLegacyTransaction` is true (e.g., fail the test with a clear message if not) so `asLegacyTransaction` presence in the swap-instructions payload is verified.internal/tools/solana_tx_test.go (1)
439-493: Test name and assertions are slightly out of sync.
TestBuildSolanaSwap_DefaultSlippageAndInputMintvalidates default input mint, but not slippage. Either assert slippage explicitly or rename the test to avoid ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tools/solana_tx_test.go` around lines 439 - 493, The test TestBuildSolanaSwap_DefaultSlippageAndInputMint name claims to cover default slippage but contains no assertion for it; update the test (or rename it) so name and assertions match: either add an explicit check against tx.TxDetails["slippage"] to verify the expected default slippage value used by handleBuildSolanaSwap (read the default from the handler or use the known constant) or rename the test to something like TestBuildSolanaSwap_DefaultInputMint to reflect it only asserts the default input_mint; locate the assertion block after unmarshalling into result and modify the checks on tx := result.Transactions[0] and tx.TxDetails accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/jupiter/client.go`:
- Around line 30-37: BuildSwapTransaction currently assumes c.rpcClient is
non-nil and dereferences it, which can panic if the caller constructed Client
without an rpc client; add a defensive nil-check at the start of
BuildSwapTransaction (inspect method BuildSwapTransaction on type Client) that
returns a clear error if c.rpcClient == nil (or otherwise aborts the operation),
so the method fails gracefully instead of causing a runtime panic; you may also
consider updating NewClient or its callers to enforce/validate rpcClient but
implement the nil guard inside BuildSwapTransaction immediately.
---
Duplicate comments:
In `@internal/jupiter/client.go`:
- Around line 160-171: In Client.GetQuote, guard against a nil or non-positive
amount before calling amount.String(): validate that the amount parameter is not
nil and that amount.Cmp(big.NewInt(0)) > 0, and return a descriptive error if
the check fails; only after that proceed to build params (where amount.String()
is used) so callers get a controlled error instead of a panic from
amount.String().
- Around line 194-200: The swapRequest payload currently created in the
swapRequest construction (symbol: swapRequest and fields WrapAndUnwrapSol,
UseSharedAccounts, DynamicComputeUnitLimit) does not request legacy transaction
instructions; update the payload to explicitly request legacy swap instructions
by adding the Jupiter legacy flag (e.g., set the appropriate field such as
UseLegacyTransaction / UseLegacyInstructions / UseLegacyTransactions to true)
when building the swapRequest so Jupiter returns legacy txn instructions and
avoids ALT-related rejections; modify the swapRequest construction site where
UserPublicKey and QuoteResponse are set to include that legacy flag.
---
Nitpick comments:
In `@internal/jupiter/client_test.go`:
- Around line 180-190: The test decoding the swap request must also assert that
the AsLegacyTransaction flag is set to true to catch regressions; after decoding
into the local variable `body` (type `swapRequest`) and the existing assertions
for `WrapAndUnwrapSol` and `DynamicComputeUnitLimit`, add an assertion that
`body.AsLegacyTransaction` is true (e.g., fail the test with a clear message if
not) so `asLegacyTransaction` presence in the swap-instructions payload is
verified.
In `@internal/tools/solana_tx_test.go`:
- Around line 439-493: The test TestBuildSolanaSwap_DefaultSlippageAndInputMint
name claims to cover default slippage but contains no assertion for it; update
the test (or rename it) so name and assertions match: either add an explicit
check against tx.TxDetails["slippage"] to verify the expected default slippage
value used by handleBuildSolanaSwap (read the default from the handler or use
the known constant) or rename the test to something like
TestBuildSolanaSwap_DefaultInputMint to reflect it only asserts the default
input_mint; locate the assertion block after unmarshalling into result and
modify the checks on tx := result.Transactions[0] and tx.TxDetails accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/jupiter/client.gointernal/jupiter/client_test.gointernal/tools/solana_tx_test.go
webpiratt
approved these changes
Feb 27, 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.
Closes vultisig/recipes#544
Summary by CodeRabbit
New Features
Tests