fix: prevent duplicate private sync pr in release script#12159
Closed
gomesalexandre wants to merge 5 commits intodevelopfrom
Closed
fix: prevent duplicate private sync pr in release script#12159gomesalexandre wants to merge 5 commits intodevelopfrom
gomesalexandre wants to merge 5 commits intodevelopfrom
Conversation
* chore: prerelease v1.1016.0 (#12127) * fix: copy patches dir in public-api Dockerfile for pnpm install pnpm requires the patches directory during install (not just scripts) because patched dependencies are referenced in the lockfile. The --ignore-scripts flag alone doesn't prevent pnpm from reading patch files during dependency resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add .railwayignore to reduce snapshot size for public-api deploys Railway's "Failed to snapshot repository" error is caused by the repo being too large (~74MB tracked files). This excludes frontend source, images, tests, and other files not needed for the public-api Dockerfile build from Railway's pre-build snapshot. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert "fix: add .railwayignore to reduce snapshot size for public-api deploys" This reverts commit 473f2d3. * Revert "fix: copy patches dir in public-api Dockerfile for pnpm install" This reverts commit a0618c1. * fix: unrug Railway CI - copy patches dir and add .railwayignore (#12099) * fix: unrug public-api Railway CI - copy patches dir and add .railwayignore Two fixes for Railway deployment failures: 1. Copy patches/ directory in Dockerfile before pnpm install - pnpm requires patch files during install even with --ignore-scripts because patched dependencies are referenced in the lockfile. 2. Add .railwayignore to reduce repo snapshot size - Railway's "Failed to snapshot repository" error was caused by the repo being ~74MB of tracked files. Excludes frontend source, images, tests, and other files not needed for the public-api Docker build. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add patches dir to swap-widget Dockerfile and update .railwayignore The swap-widget Dockerfile had the same missing patches/ dir bug as public-api — pnpm install fails with ENOENT for patched dependencies. Also add swap-widget *.md exception to .railwayignore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: use copy package-import-method in Docker to avoid pnpm ENOENT (#12100) * fix: use copy package-import-method in Docker to avoid pnpm ENOENT pnpm's default hard-link strategy fails intermittently on Docker's overlay filesystem with "ENOENT: rename _tmp -> secp256k1". Setting package-import-method=copy via env var (Docker-only, doesn't affect local dev .npmrc) fixes the atomic rename race condition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add retry loop for pnpm install in Docker builds The ENOENT rename failure persists even with package-import-method=copy because pnpm's hoisting/linking phase still does atomic renames that race on Docker's overlay filesystem. Since it's intermittent and all packages are cached in the store after the first attempt, a retry with clean node_modules is fast and reliable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: restrict swap-widget tsconfig types to prevent auto-discovery Without an explicit `types` field, TypeScript auto-discovers all @types/* packages hoisted to root node_modules. The deprecated stub @types/ethereumjs-util (from hdwallet-ledger devDeps) has no .d.ts files, causing TS2688 during Docker builds. Restricting to ["vite/client"] matches the pattern used by the root tsconfig. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: speed up stuck btc transactions via rbf (#11885) * fix: railway build with pnpm (#12107) * feat: thorchain solana lp integration (#12037) * fix: narrow YieldExplainers action prop type to remove unused claim variant (#12073) * fix: always refresh account balances after swap completion (#12106) * chore: unify claude and codex instruction entrypoints (#12119) * feat: idempotent release script state machine (#12110) * feat: bebop solana swapper take 2 (#12111) * chore: remove unused @chainflip/rpc and @chainflip/extrinsics deps (#12109) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gomes-bot <contact@0xgom.es> Co-authored-by: kevin <35275952+kaladinlight@users.noreply.github.com> Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> Co-authored-by: NeOMakinG <14963751+NeOMakinG@users.noreply.github.com> * fix: bebop solana signing + thorchain solana lp compute budget (#12132) * fix: bebop solana signing + reject amm-routed quotes (#12147) * fix: cherry-pick #12148 - bebop solana ghost tx + malformed amm routes (#12151) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gomes-bot <contact@0xgom.es> Co-authored-by: kevin <35275952+kaladinlight@users.noreply.github.com> Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> Co-authored-by: NeOMakinG <14963751+NeOMakinG@users.noreply.github.com>
chore: release v1.1016.0 (#12128) * chore: prerelease v1.1016.0 (#12127) * fix: copy patches dir in public-api Dockerfile for pnpm install pnpm requires the patches directory during install (not just scripts) because patched dependencies are referenced in the lockfile. The --ignore-scripts flag alone doesn't prevent pnpm from reading patch files during dependency resolution. * fix: add .railwayignore to reduce snapshot size for public-api deploys Railway's "Failed to snapshot repository" error is caused by the repo being too large (~74MB tracked files). This excludes frontend source, images, tests, and other files not needed for the public-api Dockerfile build from Railway's pre-build snapshot. * Revert "fix: add .railwayignore to reduce snapshot size for public-api deploys" This reverts commit 473f2d3. * Revert "fix: copy patches dir in public-api Dockerfile for pnpm install" This reverts commit a0618c1. * fix: unrug Railway CI - copy patches dir and add .railwayignore (#12099) * fix: unrug public-api Railway CI - copy patches dir and add .railwayignore Two fixes for Railway deployment failures: 1. Copy patches/ directory in Dockerfile before pnpm install - pnpm requires patch files during install even with --ignore-scripts because patched dependencies are referenced in the lockfile. 2. Add .railwayignore to reduce repo snapshot size - Railway's "Failed to snapshot repository" error was caused by the repo being ~74MB of tracked files. Excludes frontend source, images, tests, and other files not needed for the public-api Docker build. * fix: add patches dir to swap-widget Dockerfile and update .railwayignore The swap-widget Dockerfile had the same missing patches/ dir bug as public-api — pnpm install fails with ENOENT for patched dependencies. Also add swap-widget *.md exception to .railwayignore. --------- * fix: use copy package-import-method in Docker to avoid pnpm ENOENT (#12100) * fix: use copy package-import-method in Docker to avoid pnpm ENOENT pnpm's default hard-link strategy fails intermittently on Docker's overlay filesystem with "ENOENT: rename _tmp -> secp256k1". Setting package-import-method=copy via env var (Docker-only, doesn't affect local dev .npmrc) fixes the atomic rename race condition. * fix: add retry loop for pnpm install in Docker builds The ENOENT rename failure persists even with package-import-method=copy because pnpm's hoisting/linking phase still does atomic renames that race on Docker's overlay filesystem. Since it's intermittent and all packages are cached in the store after the first attempt, a retry with clean node_modules is fast and reliable. * fix: restrict swap-widget tsconfig types to prevent auto-discovery Without an explicit `types` field, TypeScript auto-discovers all @types/* packages hoisted to root node_modules. The deprecated stub @types/ethereumjs-util (from hdwallet-ledger devDeps) has no .d.ts files, causing TS2688 during Docker builds. Restricting to ["vite/client"] matches the pattern used by the root tsconfig. --------- * feat: speed up stuck btc transactions via rbf (#11885) * fix: railway build with pnpm (#12107) * feat: thorchain solana lp integration (#12037) * fix: narrow YieldExplainers action prop type to remove unused claim variant (#12073) * fix: always refresh account balances after swap completion (#12106) * chore: unify claude and codex instruction entrypoints (#12119) * feat: idempotent release script state machine (#12110) * feat: bebop solana swapper take 2 (#12111) * chore: remove unused @chainflip/rpc and @chainflip/extrinsics deps (#12109) --------- * fix: bebop solana signing + thorchain solana lp compute budget (#12132) * fix: bebop solana signing + reject amm-routed quotes (#12147) * fix: cherry-pick #12148 - bebop solana ghost tx + malformed amm routes (#12151) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gomes-bot <contact@0xgom.es> Co-authored-by: kevin <35275952+kaladinlight@users.noreply.github.com> Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> Co-authored-by: NeOMakinG <14963751+NeOMakinG@users.noreply.github.com>
merged_untagged case was creating a private sync PR without checking if one already existed, unlike tagged_private_stale which had the guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces comprehensive infrastructure for documentation and agent tooling, implements Solana serialized transaction signing across multiple wallet adapters, adds Bebop Solana swapping capabilities, implements Bitcoin RBF transaction speed-up functionality, and updates Dockerfile/pnpm configurations alongside new test coverage for release workflows and speed-up utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActionCenter as Action Center
participant SpeedUpModal as SpeedUp Modal
participant Wallet as Wallet
participant BlockchainAdapter as Blockchain Adapter
participant Backend as Backend/Network
User->>ActionCenter: Click "Speed Up" on pending TX
ActionCenter->>SpeedUpModal: Open with original TX hash
SpeedUpModal->>Backend: Fetch original TX details & UTXOs
Backend-->>SpeedUpModal: Original TX data
SpeedUpModal->>SpeedUpModal: Reconstruct inputs/outputs, calculate fee rates
User->>SpeedUpModal: Select new fee rate multiplier
SpeedUpModal->>SpeedUpModal: Build replacement TX with new fee
SpeedUpModal->>Wallet: Sign replacement TX (RBF)
Wallet-->>SpeedUpModal: Signature
SpeedUpModal->>BlockchainAdapter: Broadcast replacement TX
BlockchainAdapter-->>Backend: Submit to mempool
Backend-->>SpeedUpModal: TX hash/confirmation
SpeedUpModal->>ActionCenter: Update action state (Replaced)
SpeedUpModal-->>User: Display success with fee change
sequenceDiagram
participant App as Application
participant BebopSwapper as Bebop Swapper
participant Wallet as Wallet/Signer
participant BebopService as Bebop Service
participant Solana as Solana Network
App->>BebopSwapper: Get Solana trade quote
BebopSwapper->>BebopService: Fetch Solana quote
BebopService-->>BebopSwapper: Quote with serialized TX
BebopSwapper->>App: Return quote with bebopSolanaSerializedTx
App->>BebopSwapper: Execute trade (getMessage)
BebopSwapper-->>App: Return unsigned serialized message
App->>Wallet: Sign serialized transaction
Wallet-->>App: Signatures array (base64)
App->>BebopSwapper: Execute message (sign + broadcast)
BebopSwapper->>Wallet: Sign via solanaSignSerializedTx
Wallet-->>BebopSwapper: Signed TX with signatures
BebopSwapper->>BebopService: Submit order with signature
BebopService->>Solana: Broadcast signed TX
Solana-->>BebopService: TX hash
BebopService-->>BebopSwapper: Order confirmed
BebopSwapper-->>App: Return TX hash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Author
|
closing to recreate with clean diff |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (3)
src/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsx (1)
111-111: UseuseErrorToastfor the new modal error toasts.These new failure paths are hand-rolled with
useToast(), which skips the app's standardized error-toast handling for translated, user-facing errors.As per coding guidelines, "ALWAYS use
useErrorToasthook for displaying errors with translated error messages and handle different error types appropriately".Also applies to: 378-385, 688-695
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsx` at line 111, The component currently creates a generic toast instance via the variable "toast" using useToast(); replace that with the app standard useErrorToast hook by importing useErrorToast and calling it instead of useToast inside SpeedUpModal, then update all error display sites that use toast (the new modal failure paths and the other occurrences where toast is used) to call the error-toast helper (e.g., useErrorToast(error, { title: t('...') }) or the hook's API) so translated, standardized error toasts are produced; specifically swap useToast -> useErrorToast and replace direct toast.error(...) usage with the appropriate useErrorToast invocation in this component.packages/swapper/src/types.ts (1)
432-433: Make the Bebop Solana metadata all-or-nothing.These values are consumed together, but the step type currently allows only one of them to be populated. Grouping them under a single optional object keeps this new path impossible to half-populate.
♻️ Suggested shape
- bebopSolanaSerializedTx?: string - bebopQuoteId?: string + bebopSolanaMessage?: { + serializedTx: string + quoteId: string + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/types.ts` around lines 432 - 433, Currently the Bebop Solana metadata fields bebopSolanaSerializedTx and bebopQuoteId are separate and can be partially populated; change the type so they are nested together under a single optional object (e.g., bebopSolana?: { serializedTx: string; quoteId: string }) so consumers of these values always get both or none; update the declaration in types.ts (replace the two top-level optional fields with one optional object) and adjust any code that accesses bebopSolanaSerializedTx or bebopQuoteId to access bebopSolana.serializedTx and bebopSolana.quoteId.src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts (1)
27-186: Add explicit return types to the exported helpers.This new utility module exposes a public API entirely via inference. Please annotate the returns now so later internal refactors don’t silently change the exported surface.
As per coding guidelines, `**/*.{ts,tsx}`: ALWAYS use explicit types for function parameters and return values in TypeScript.📝 Example
-export const toSats = (value?: TxValue) => { +export const toSats = (value?: TxValue): BigNumber => { @@ -export const getTxFeeSats = (tx: SpeedUpTxLike) => { +export const getTxFeeSats = (tx: SpeedUpTxLike): BigNumber => { @@ -export const resolveVinVoutIndex = ({ +export const resolveVinVoutIndex = ({ vinVout, vinValue, vinAddress, prevTxVouts, }: { vinVout?: string | number | null vinValue?: TxValue vinAddress?: string prevTxVouts: VoutLike[] -}) => { +}): number | undefined => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts` around lines 27 - 186, Add explicit return type annotations to all exported helpers so the public API is stable: annotate toSats, getTxVsize, getTxVirtualBytes, getTxFeeSats, getTxFeeRateSatPerVbPrecise, getTxFeeRateSatPerVb, getDisplayFeeRateSatPerVb, getDisplayFeeRateSatPerVbPrecise to return the appropriate BigNumber type used by bn/bnOrZero; annotate resolveVinVoutIndex to return number | undefined, getTxIdFromHex to return string | undefined, and keep the type predicate on isLikelyBitcoinTxId; update the function signatures (not bodies) to include these explicit return types and import the correct BigNumber type if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.railwayignore:
- Around line 10-11: The negated include
'!public/generated/generatedAssetData.json' won't work because the parent
'public/' is ignored; update the ignore rules so the parent directory is
unignored before re-including the file: replace or augment the patterns so
'public/' is not globally ignored (e.g., remove 'public/' or add an explicit
unignore line '!public/' before '!public/generated/generatedAssetData.json') or
instead narrow the ignore (e.g., ignore 'public/*' but add
'!public/generated/generatedAssetData.json') so the generatedAssetData.json file
is actually included in the Railway snapshot.
In `@packages/hdwallet-gridplus/src/solana.ts`:
- Around line 103-108: The catch is too broad: change the try/catch around
transaction.serialize() to catch the thrown error as a variable (e.g., err) and
only fall back to msg.serializedTx when the error clearly indicates the expected
"missing co-signer" condition (inspect err.message or err.code for that specific
case); for any other serialize errors rethrow the error so the real signing
failure surfaces. Update the block around transaction.serialize(), serialized,
and msg.serializedTx accordingly so unrelated exceptions are not swallowed.
- Around line 69-72: solanaSignTx currently forwards msg.pubKey into
solanaGetAddress but the implementation always calls client.getAddresses(),
adding an unnecessary device call; preserve the bypass by short-circuiting when
msg.pubKey is present — either update solanaGetAddress to immediately return the
provided pubKey (and derived address) when a pubKey argument exists or modify
solanaSignTx to skip calling client.getAddresses and construct/return the
address from msg.pubKey directly; ensure you reference solanaSignTx,
solanaGetAddress, msg.pubKey, client.getAddresses and the local address variable
so the change avoids the extra device call and maintains the existing bypass
behavior.
In `@packages/hdwallet-ledger/src/solana.ts`:
- Around line 73-78: The current blanket catch around transaction.serialize()
swallows any error; change it to only fallback to msg.serializedTx for the
specific "missing cosigner"/incomplete-signatures case and rethrow other errors.
Concretely, replace the empty catch with catch (err) { if
(isMissingCosignerError(err)) { serialized = msg.serializedTx } else { throw err
} } and add a small helper isMissingCosignerError(err) that checks the error
type/message (e.g. looks for known Ledger/Solana text like "missing", "not all
signers", or provider-specific error codes) so unrelated serialize() failures
are not hidden; keep using transaction.serialize(), serialized and
msg.serializedTx as in the diff.
In `@packages/hdwallet-native/src/solana.ts`:
- Around line 88-95: The catch is too broad around signedTransaction.serialize()
and hides real errors; update the catch in the serialize block (around
signedTransaction.serialize() / msg.serializedTx) to rethrow unexpected
exceptions and only use the msg.serializedTx fallback when the failure is
clearly due to missing cosigner signatures—e.g., inspect
signedTransaction.signatures (or the caught error message/name) and if any
signature is null/undefined (or the error explicitly indicates missing
signature), then set serialized = msg.serializedTx; otherwise throw the original
error so real serialize failures are not swallowed.
In `@packages/hdwallet-trezor/src/solana.ts`:
- Around line 116-121: The current try/catch around transaction.serialize()
swallows all errors and falls back to msg.serializedTx; change it so you only
use the fallback when the caught error is the expected "missing cosigner/partial
signature" case and rethrow for any other exception. In practice, inspect the
caught error (e.g., err.message) after transaction.serialize() fails and if it
matches the known missing-cosigner signature text then set serialized =
msg.serializedTx, otherwise throw the error so the real serialize/signing
failure surfaces; reference the transaction.serialize() call, the serialized
variable, and msg.serializedTx when locating and updating the code.
In `@packages/hdwallet-vultisig/src/solana.ts`:
- Around line 40-45: The try/catch around signedTransaction.serialize() is
swallowing all exceptions and returning msg.serializedTx even for unexpected
serialize failures; update the catch to only handle the specific
missing-cosigner case (or inspect the thrown error's name/message for the
expected "missing cosigner" condition) and rethrow any other errors so real
serialization/signing failures are not masked; keep the existing fallback to
msg.serializedTx only when the error matches the missing-cosigner condition
(referencing signedTransaction.serialize(), the serialized variable, and
msg.serializedTx).
In `@packages/public-api/Dockerfile`:
- Around line 58-61: The for-loop attempting retries ("for i in 1 2 3; do ...
done") masks a final failure because the trailing "rm -rf node_modules" becomes
the loop's exit status; change the loop so that on pnpm install failure it
removes node_modules but then fails the build immediately (e.g., by exiting
non-zero) instead of falling through — target the pnpm install invocation and
the node_modules cleanup in the loop body (symbols: "pnpm install",
"node_modules", the retry loop "for i in 1 2 3; do ... done") and ensure the
final unsuccessful attempt causes an explicit non-zero exit rather than letting
the cleanup command be the last status.
In `@packages/swap-widget/Dockerfile`:
- Around line 58-61: The retry loop around pnpm install currently swallows
failures because the final command executed can be rm -rf node_modules which
returns 0; change the logic so the RUN fails if all retries exhaust: after the
for loop that runs "pnpm install ... && break || rm -rf node_modules", add a
check that verifies installation succeeded (e.g., test for node_modules or check
last exit status) and exit non-zero with an error message if not; alternatively
append a final "|| exit 1" or explicit failure check after the loop to ensure
the Docker build fails when pnpm install did not succeed.
In `@packages/swapper/src/swappers/BebopSwapper/endpoints.ts`:
- Around line 99-113: Mark getUnsignedSolanaMessage as an async function so it
always returns a rejected Promise on errors rather than throwing synchronously:
change its declaration to async getUnsignedSolanaMessage(...) and remove
Promise.resolve, returning the object directly; keep the existing precondition
checks using isExecutableTradeQuote and getExecutableTradeStep and the same
error throws for missing bebopSolanaSerializedTx/bebopQuoteId so they become
rejected promises instead of synchronous exceptions.
In `@pnpm-workspace.yaml`:
- Line 3: Revert the strict peer enforcement change by removing or setting the
strictPeerDependencies entry back to the repo baseline (disable it) so pnpm
behaves permissively like the migration baseline; specifically update the
pnpm-workspace.yaml entry for strictPeerDependencies (remove the key or set it
to false) to restore the previous non-strict behavior.
In `@RELEASE.md`:
- Around line 28-46: The two unlabeled fenced code blocks in RELEASE.md should
be changed to use a language label to satisfy MD040: update both occurrences of
the triple-backtick code fences (the first block showing "idle (no prerelease)
..." and the second "idle ... cherry-pick commits...") to start with ```text
instead of ``` so both blocks are labeled as text for markdownlint.
In `@scripts/release.ts`:
- Around line 766-793: In the merged_untagged hotfix branch (case
'merged_untagged'), mirror the regular release flow’s guard: before calling
createPr for the private sync PR, check for an existing open private sync PR
(the same logic used earlier around lines 613-630) and if found log and reuse
its URL instead of creating a duplicate; otherwise call createPr as currently
implemented. Ensure you reference nextVersion in the search/match, update the
console.log messages to reflect whether you found an existing PR or created a
new one, and keep the backmerge creation (createPr for develop <- main)
unchanged.
In `@src/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsx`:
- Around line 93-97: ReconstructedOutput currently only tracks address and
amount so non-address outputs (OP_RETURN/memo, native scripts) are dropped when
rebuilding BTCSignTx['outputs']; update the ReconstructedOutput shape to also
carry the original output script/type (e.g. scriptPubKey or outputKind field and
rawScript bytes/string), preserve that data when you create ReconstructedOutput
instances (where outputs are parsed, e.g. in SpeedUpModal), and when you rebuild
BTCSignTx['outputs'] (the mapping at/around the code that previously filtered to
address-based outputs) emit either an address-based output or re-emit the
original non-address output using the stored script/type and amount so OP_RETURN
and other special outputs are preserved. Ensure type declarations and any
builder logic accept the raw script field so BTCSignTx['outputs'] contains the
exact original non-address outputs.
In `@src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts`:
- Around line 52-57: The helper getTxVirtualBytes currently returns tx.vsize
when >0 which discards 0.25-vB precision; change its logic to prefer weight over
vsize: call bnOrZero(tx.weight) first and if weight.gt(0) return weight.div(4),
otherwise fall back to bnOrZero(tx.vsize) and return that if >0. Keep using
bnOrZero for both fields and preserve existing return behavior when neither is
present.
- Around line 92-107: The code currently calls .integerValue() without a
rounding mode which uses ROUND_DOWN and can shrink fee rates (e.g. 1.01 -> 1);
update calls to use BigNumber.ROUND_CEIL to always round up fee bumps.
Specifically, change getTxFeeRateSatPerVb to call
getTxFeeRateSatPerVbPrecise(tx).integerValue(BigNumber.ROUND_CEIL) and change
the networkFeeRate computation in getDisplayFeeRateSatPerVb to call
bnOrZero(networkAverageFeeRateSatPerVb).integerValue(BigNumber.ROUND_CEIL) so
RBF fee bumps never round down. Use the BigNumber symbol already used elsewhere
in the file.
In `@src/components/Modals/Send/utils.ts`:
- Around line 118-145: The SPL-send fee estimation path omits memoInstruction
when contractAddress is set, so adapter.buildEstimationInstructions(...)
produces estimates that miss the memo and underestimates compute units; to fix,
after obtaining the instructions from adapter.buildEstimationInstructions({
from: account, to, tokenId: contractAddress, value }), if memoInstruction is
defined append it to that returned instruction array (or pass it via any
supported extraInstructions parameter) so the estimation includes the memo;
ensure you handle the case where buildEstimationInstructions returns undefined
and preserve the previous behavior for pure SOL sends (memo-only branch).
In
`@src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx`:
- Around line 442-444: The current check in useTradeExecution (variable result
and its signatures property) only verifies existence and misses empty arrays;
change the guard to ensure result.signatures is an array with at least one
element (and optionally contains non-null entries) before proceeding—if not,
throw the same error and prevent advancing with an empty/invalid signed payload;
update the check around the result handling in useTradeExecution to validate
Array.isArray(result.signatures) && result.signatures.length > 0 (and filter out
falsy signatures if you later iterate them).
---
Nitpick comments:
In `@packages/swapper/src/types.ts`:
- Around line 432-433: Currently the Bebop Solana metadata fields
bebopSolanaSerializedTx and bebopQuoteId are separate and can be partially
populated; change the type so they are nested together under a single optional
object (e.g., bebopSolana?: { serializedTx: string; quoteId: string }) so
consumers of these values always get both or none; update the declaration in
types.ts (replace the two top-level optional fields with one optional object)
and adjust any code that accesses bebopSolanaSerializedTx or bebopQuoteId to
access bebopSolana.serializedTx and bebopSolana.quoteId.
In `@src/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsx`:
- Line 111: The component currently creates a generic toast instance via the
variable "toast" using useToast(); replace that with the app standard
useErrorToast hook by importing useErrorToast and calling it instead of useToast
inside SpeedUpModal, then update all error display sites that use toast (the new
modal failure paths and the other occurrences where toast is used) to call the
error-toast helper (e.g., useErrorToast(error, { title: t('...') }) or the
hook's API) so translated, standardized error toasts are produced; specifically
swap useToast -> useErrorToast and replace direct toast.error(...) usage with
the appropriate useErrorToast invocation in this component.
In `@src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts`:
- Around line 27-186: Add explicit return type annotations to all exported
helpers so the public API is stable: annotate toSats, getTxVsize,
getTxVirtualBytes, getTxFeeSats, getTxFeeRateSatPerVbPrecise,
getTxFeeRateSatPerVb, getDisplayFeeRateSatPerVb,
getDisplayFeeRateSatPerVbPrecise to return the appropriate BigNumber type used
by bn/bnOrZero; annotate resolveVinVoutIndex to return number | undefined,
getTxIdFromHex to return string | undefined, and keep the type predicate on
isLikelyBitcoinTxId; update the function signatures (not bodies) to include
these explicit return types and import the correct BigNumber type if necessary.
🪄 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: CHILL
Plan: Pro
Run ID: 3ab31e02-f867-4b11-b50b-92bea645b580
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (72)
.claude/guidelines/beads-rules.md.claude/guidelines/global-rules.md.claude/guidelines/pr-rules.md.claude/guidelines/project-rules.md.claude/programming-guidelines.md.claude/test-scenarios/README.md.codex/config.toml.codex/skills.npmrc.railwayignoreAGENTS.mdAGENTS.mdCLAUDE.mdRELEASE.mde2e/fixtures/bebop-solana-swap.yamlpackage.jsonpackages/chain-adapters/src/utxo/UtxoBaseAdapter.tspackages/hdwallet-core/src/solana.tspackages/hdwallet-gridplus/src/gridplus.tspackages/hdwallet-gridplus/src/solana.tspackages/hdwallet-ledger/src/ledger.tspackages/hdwallet-ledger/src/solana.tspackages/hdwallet-native/src/solana.tspackages/hdwallet-phantom/src/phantom.tspackages/hdwallet-phantom/src/solana.tspackages/hdwallet-trezor/src/solana.tspackages/hdwallet-trezor/src/trezor.tspackages/hdwallet-vultisig/src/solana.tspackages/hdwallet-vultisig/src/vultisig.tspackages/public-api/Dockerfilepackages/public-api/Dockerfile.dockerignorepackages/swap-widget/Dockerfilepackages/swapper/src/swappers/BebopSwapper/BebopSwapper.tspackages/swapper/src/swappers/BebopSwapper/endpoints.tspackages/swapper/src/swappers/BebopSwapper/executeSolanaMessage.tspackages/swapper/src/swappers/BebopSwapper/getBebopSolanaTradeQuote/getBebopSolanaTradeQuote.tspackages/swapper/src/swappers/BebopSwapper/getBebopSolanaTradeRate/getBebopSolanaTradeRate.tspackages/swapper/src/swappers/BebopSwapper/types.tspackages/swapper/src/swappers/BebopSwapper/utils/fetchFromBebop.tspackages/swapper/src/swappers/BebopSwapper/utils/helpers/helpers.tspackages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.tspackages/swapper/src/thorchain-utils/solana/constants.tspackages/swapper/src/types.tspnpm-workspace.yamlscripts/release.test.tsscripts/release.tssrc/assets/translations/en/main.jsonsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/Notifications/GenericTransactionNotification.tsxsrc/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsxsrc/components/Layout/Header/ActionCenter/components/speedUpUtils.test.tssrc/components/Layout/Header/ActionCenter/components/speedUpUtils.tssrc/components/Modals/QrCode/Form.tsxsrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/hooks/useCompleteSendFlow.tsxsrc/components/Modals/Send/hooks/useFormSend/useFormSend.tsxsrc/components/Modals/Send/utils.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/TransactionHistoryRows/TransactionCommon.tsxsrc/hooks/useActionCenterSubscribers/useSendActionSubscriber.tsxsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/hooks/useLocaleFormatter/useLocaleFormatter.test.tsxsrc/lib/tradeExecution.tssrc/lib/utils/thorchain/index.tssrc/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/pages/ThorChainLP/components/LpType.tsxsrc/pages/Yields/components/YieldExplainers.tsxsrc/state/slices/actionSlice/types.ts
💤 Files with no reviewable changes (3)
- package.json
- .npmrc
- packages/public-api/Dockerfile.dockerignore
Comment on lines
+10
to
+11
| public/ | ||
| !public/generated/generatedAssetData.json |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Railway's .railwayignoreuse gitignore-compatible negation rules where!child/file under an ignored parent requires unignoring parent directories first?
💡 Result:
Yes—Railway’s .railwayignore is treated as gitignore-style ignore patterns, including ! negation, and it follows the same constraint that you can’t re-include a file if a parent directory is excluded (so you must unignore parent directories first). Railway’s own ignore parser documents this gitignore rule explicitly. [1], [2]
So if you have something like:
parent/
!parent/child/fileyou generally need to also unignore the intermediate directories, e.g.:
parent/
!parent/child/
!parent/child/file(or !parent/ etc., depending on what you ignored). [2]
Sources: Railway CLI docs note .railwayignore is respected during railway up uploads [1], and the CLI’s ignore implementation references the gitignore negation rule and the “parent directory excluded” limitation [2].
[1] (docs.railway.com)
[2] (github.com)
Citations:
- 1: https://docs.railway.com/cli/up?utm_source=openai
- 2: https://github.com/railwayapp/cli/blob/v1.8.4/gateway/gitignore.go
Negated asset include will not work under an ignored parent directory
Line 10 ignores public/, but Line 11's attempt to re-include a nested file with !public/generated/generatedAssetData.json will not work. With gitignore-style semantics (which Railway's .railwayignore follows), you cannot re-include a child file under an ignored parent directory without first unignoring the parent. The asset will be excluded from the Railway snapshot.
Suggested pattern fix
-public/
-!public/generated/generatedAssetData.json
+public/
+!public/
+!public/generated/generatedAssetData.json📝 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.
Suggested change
| public/ | |
| !public/generated/generatedAssetData.json | |
| public/ | |
| !public/ | |
| !public/generated/generatedAssetData.json |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.railwayignore around lines 10 - 11, The negated include
'!public/generated/generatedAssetData.json' won't work because the parent
'public/' is ignored; update the ignore rules so the parent directory is
unignored before re-including the file: replace or augment the patterns so
'public/' is not globally ignored (e.g., remove 'public/' or add an explicit
unignore line '!public/' before '!public/generated/generatedAssetData.json') or
instead narrow the ignore (e.g., ignore 'public/*' but add
'!public/generated/generatedAssetData.json') so the generatedAssetData.json file
is actually included in the Railway snapshot.
Comment on lines
+69
to
+72
| const address = await solanaGetAddress(client, { | ||
| addressNList: msg.addressNList, | ||
| }) | ||
| if (!address) throw new Error('Failed to get Solana address') |
Contributor
There was a problem hiding this comment.
Preserve the existing pubKey bypass.
solanaSignTx() forwards msg.pubKey into solanaGetAddress(), but this new path always round-trips through client.getAddresses(). That adds an avoidable device call and failure point even when the caller already supplied the signer pubkey.
🔧 Keep the bypass behavior consistent
const address = await solanaGetAddress(client, {
addressNList: msg.addressNList,
+ pubKey: msg.pubKey,
})📝 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.
Suggested change
| const address = await solanaGetAddress(client, { | |
| addressNList: msg.addressNList, | |
| }) | |
| if (!address) throw new Error('Failed to get Solana address') | |
| const address = await solanaGetAddress(client, { | |
| addressNList: msg.addressNList, | |
| pubKey: msg.pubKey, | |
| }) | |
| if (!address) throw new Error('Failed to get Solana address') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/hdwallet-gridplus/src/solana.ts` around lines 69 - 72, solanaSignTx
currently forwards msg.pubKey into solanaGetAddress but the implementation
always calls client.getAddresses(), adding an unnecessary device call; preserve
the bypass by short-circuiting when msg.pubKey is present — either update
solanaGetAddress to immediately return the provided pubKey (and derived address)
when a pubKey argument exists or modify solanaSignTx to skip calling
client.getAddresses and construct/return the address from msg.pubKey directly;
ensure you reference solanaSignTx, solanaGetAddress, msg.pubKey,
client.getAddresses and the local address variable so the change avoids the
extra device call and maintains the existing bypass behavior.
Comment on lines
+103
to
+108
| let serialized: string | ||
| try { | ||
| serialized = Buffer.from(transaction.serialize()).toString('base64') | ||
| } catch { | ||
| serialized = msg.serializedTx | ||
| } |
Contributor
There was a problem hiding this comment.
Don't swallow unrelated serialize() failures.
This fallback runs for any exception, not just the missing-cosigner case. If serialize() fails unexpectedly, the method returns the original serializedTx alongside fresh signatures and hides the real signing error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/hdwallet-gridplus/src/solana.ts` around lines 103 - 108, The catch
is too broad: change the try/catch around transaction.serialize() to catch the
thrown error as a variable (e.g., err) and only fall back to msg.serializedTx
when the error clearly indicates the expected "missing co-signer" condition
(inspect err.message or err.code for that specific case); for any other
serialize errors rethrow the error so the real signing failure surfaces. Update
the block around transaction.serialize(), serialized, and msg.serializedTx
accordingly so unrelated exceptions are not swallowed.
Comment on lines
+73
to
+78
| let serialized: string | ||
| try { | ||
| serialized = Buffer.from(transaction.serialize()).toString('base64') | ||
| } catch { | ||
| serialized = msg.serializedTx | ||
| } |
Contributor
There was a problem hiding this comment.
Don't swallow unrelated serialize() failures.
This fallback runs for any exception, not just the missing-cosigner case. If serialize() fails unexpectedly, the method returns the original serializedTx alongside fresh signatures and hides the real signing error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/hdwallet-ledger/src/solana.ts` around lines 73 - 78, The current
blanket catch around transaction.serialize() swallows any error; change it to
only fallback to msg.serializedTx for the specific "missing
cosigner"/incomplete-signatures case and rethrow other errors. Concretely,
replace the empty catch with catch (err) { if (isMissingCosignerError(err)) {
serialized = msg.serializedTx } else { throw err } } and add a small helper
isMissingCosignerError(err) that checks the error type/message (e.g. looks for
known Ledger/Solana text like "missing", "not all signers", or provider-specific
error codes) so unrelated serialize() failures are not hidden; keep using
transaction.serialize(), serialized and msg.serializedTx as in the diff.
Comment on lines
+88
to
+95
| let serialized: string | ||
| try { | ||
| serialized = Buffer.from(signedTransaction.serialize()).toString('base64') | ||
| } catch { | ||
| // Partial signing - serialize the message without signature verification | ||
| // For co-signed txs (e.g. gasless Bebop), not all sigs are present yet | ||
| serialized = msg.serializedTx | ||
| } |
Contributor
There was a problem hiding this comment.
Don't swallow unrelated serialize() failures.
This fallback runs for any exception, not just the missing-cosigner case. If serialize() fails unexpectedly, the method returns the original serializedTx alongside fresh signatures and hides the real signing error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/hdwallet-native/src/solana.ts` around lines 88 - 95, The catch is
too broad around signedTransaction.serialize() and hides real errors; update the
catch in the serialize block (around signedTransaction.serialize() /
msg.serializedTx) to rethrow unexpected exceptions and only use the
msg.serializedTx fallback when the failure is clearly due to missing cosigner
signatures—e.g., inspect signedTransaction.signatures (or the caught error
message/name) and if any signature is null/undefined (or the error explicitly
indicates missing signature), then set serialized = msg.serializedTx; otherwise
throw the original error so real serialize failures are not swallowed.
Comment on lines
+93
to
+97
| type ReconstructedOutput = { | ||
| address?: string | ||
| amount: string | ||
| isChange: boolean | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve non-address outputs in the replacement transaction.
ReconstructedOutput only carries address and amount, and Line 537 rebuilds outputs from addressed recipients only. Any BTC send that included an OP_RETURN/memo output will lose that output during speed-up, which changes the transaction semantics and can orphan protocol deposits.
Carry the original script/output kind through reconstruction and re-emit it when building BTCSignTx['outputs'], instead of filtering everything down to address-based outputs.
Also applies to: 230-240, 537-543
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Layout/Header/ActionCenter/components/SpeedUpModal.tsx` around
lines 93 - 97, ReconstructedOutput currently only tracks address and amount so
non-address outputs (OP_RETURN/memo, native scripts) are dropped when rebuilding
BTCSignTx['outputs']; update the ReconstructedOutput shape to also carry the
original output script/type (e.g. scriptPubKey or outputKind field and rawScript
bytes/string), preserve that data when you create ReconstructedOutput instances
(where outputs are parsed, e.g. in SpeedUpModal), and when you rebuild
BTCSignTx['outputs'] (the mapping at/around the code that previously filtered to
address-based outputs) emit either an address-based output or re-emit the
original non-address output using the stored script/type and amount so OP_RETURN
and other special outputs are preserved. Ensure type declarations and any
builder logic accept the raw script field so BTCSignTx['outputs'] contains the
exact original non-address outputs.
Comment on lines
+52
to
+57
| export const getTxVirtualBytes = (tx: SpeedUpTxLike) => { | ||
| const vsize = bnOrZero(tx.vsize) | ||
| if (vsize.gt(0)) return vsize | ||
|
|
||
| const weight = bnOrZero(tx.weight) | ||
| if (weight.gt(0)) return weight.div(4) |
Contributor
There was a problem hiding this comment.
Prefer weight over vsize in the precise helper.
getTxVirtualBytes() is the denominator for the Precise fee-rate helpers. Returning rounded vsize first throws away the 0.25-vB precision that weight / 4 would preserve when both fields are present.
🐛 Proposed fix
export const getTxVirtualBytes = (tx: SpeedUpTxLike) => {
- const vsize = bnOrZero(tx.vsize)
- if (vsize.gt(0)) return vsize
-
const weight = bnOrZero(tx.weight)
if (weight.gt(0)) return weight.div(4)
+
+ const vsize = bnOrZero(tx.vsize)
+ if (vsize.gt(0)) return vsize🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts` around
lines 52 - 57, The helper getTxVirtualBytes currently returns tx.vsize when >0
which discards 0.25-vB precision; change its logic to prefer weight over vsize:
call bnOrZero(tx.weight) first and if weight.gt(0) return weight.div(4),
otherwise fall back to bnOrZero(tx.vsize) and return that if >0. Keep using
bnOrZero for both fields and preserve existing return behavior when neither is
present.
Comment on lines
+92
to
+107
| export const getTxFeeRateSatPerVb = (tx: SpeedUpTxLike) => { | ||
| return getTxFeeRateSatPerVbPrecise(tx).integerValue() | ||
| } | ||
|
|
||
| export const getDisplayFeeRateSatPerVb = ({ | ||
| tx, | ||
| networkAverageFeeRateSatPerVb, | ||
| }: { | ||
| tx: SpeedUpTxLike | ||
| networkAverageFeeRateSatPerVb?: string | number | ||
| }) => { | ||
| const txFeeRate = getTxFeeRateSatPerVb(tx) | ||
| if (txFeeRate.gt(0)) return txFeeRate | ||
|
|
||
| const networkFeeRate = bnOrZero(networkAverageFeeRateSatPerVb).integerValue() | ||
| if (networkFeeRate.gt(0)) return networkFeeRate |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file in question
fd speedUpUtils.tsRepository: shapeshift/web
Length of output: 126
🏁 Script executed:
# Check the BigNumber configuration and default settings
rg "BigNumber.*ROUND|RoundingMode|rounding" -t ts -t tsx -i --max-count=20Repository: shapeshift/web
Length of output: 85
🏁 Script executed:
# Look for BigNumber imports and configuration
rg "from.*BigNumber|import.*BigNumber" -t ts -t tsxRepository: shapeshift/web
Length of output: 85
🏁 Script executed:
# Examine the getTxFeeRateSatPerVbPrecise function
ast-grep --pattern 'getTxFeeRateSatPerVbPrecise'Repository: shapeshift/web
Length of output: 705
🏁 Script executed:
# Read the complete speedUpUtils.ts file
cat -n src/components/Layout/Header/ActionCenter/components/speedUpUtils.tsRepository: shapeshift/web
Length of output: 6227
🏁 Script executed:
# Read the test file to understand expected behavior
cat -n src/components/Layout/Header/ActionCenter/components/speedUpUtils.test.tsRepository: shapeshift/web
Length of output: 8367
🏁 Script executed:
# Search for BigNumber configuration or defaults
rg "BigNumber\\.set|config.*round|ROUND_" --type js --type ts -iRepository: shapeshift/web
Length of output: 5128
🏁 Script executed:
# Check imports in speedUpUtils.ts for BigNumber
head -30 src/components/Layout/Header/ActionCenter/components/speedUpUtils.tsRepository: shapeshift/web
Length of output: 705
🏁 Script executed:
# Check bignumber configuration in the codebase
rg "BigNumber\.set|config" src/lib/bignumber/ -A 5Repository: shapeshift/web
Length of output: 40
🏁 Script executed:
# Look for usage of getTxFeeRateSatPerVb to see downstream impact
rg "getTxFeeRateSatPerVb" --type ts --type js -B 2 -A 2Repository: shapeshift/web
Length of output: 3604
🏁 Script executed:
# Search for any BigNumber imports and see if there's global config
find src/lib/bignumber -type fRepository: shapeshift/web
Length of output: 124
🏁 Script executed:
# Read the bignumber configuration
cat -n src/lib/bignumber/bignumber.tsRepository: shapeshift/web
Length of output: 1098
🏁 Script executed:
# Check bignumber test
cat -n src/lib/bignumber/bignumber.test.tsRepository: shapeshift/web
Length of output: 4340
🏁 Script executed:
# Search for BigNumber.ROUND defaults in node_modules or online docs reference
rg "ROUND_DOWN|ROUND_CEIL|rounding.*default" --type ts --type js -iRepository: shapeshift/web
Length of output: 2225
Round integer fee rates up explicitly.
Don't rely on the ambient BigNumber rounding mode for fee bumps. The default uses ROUND_DOWN, which can turn 1.01 sat/vB into 1, causing RBF bumps to fail. Use BigNumber.ROUND_CEIL to match the existing pattern at line 39.
🐛 Proposed fix
export const getTxFeeRateSatPerVb = (tx: SpeedUpTxLike) => {
- return getTxFeeRateSatPerVbPrecise(tx).integerValue()
+ return getTxFeeRateSatPerVbPrecise(tx).integerValue(BigNumber.ROUND_CEIL)
}
@@
- const networkFeeRate = bnOrZero(networkAverageFeeRateSatPerVb).integerValue()
+ const networkFeeRate = bnOrZero(networkAverageFeeRateSatPerVb).integerValue(
+ BigNumber.ROUND_CEIL,
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Layout/Header/ActionCenter/components/speedUpUtils.ts` around
lines 92 - 107, The code currently calls .integerValue() without a rounding mode
which uses ROUND_DOWN and can shrink fee rates (e.g. 1.01 -> 1); update calls to
use BigNumber.ROUND_CEIL to always round up fee bumps. Specifically, change
getTxFeeRateSatPerVb to call
getTxFeeRateSatPerVbPrecise(tx).integerValue(BigNumber.ROUND_CEIL) and change
the networkFeeRate computation in getDisplayFeeRateSatPerVb to call
bnOrZero(networkAverageFeeRateSatPerVb).integerValue(BigNumber.ROUND_CEIL) so
RBF fee bumps never round down. Use the BigNumber symbol already used elsewhere
in the file.
Comment on lines
+118
to
145
| const memoInstruction: TransactionInstruction | undefined = memo | ||
| ? new TransactionInstruction({ | ||
| keys: [], | ||
| programId: new PublicKey(SOLANA_MEMO_PROGRAM_ID), | ||
| data: Buffer.from(memo, 'utf8'), | ||
| }) | ||
| : undefined | ||
|
|
||
| // For SPL transfers, build complete instruction set including compute budget | ||
| // For SOL transfers (pure sends i.e not e.g a Jup swap), pass no instructions to get 0 count (avoids blind signing) | ||
| // For SOL transfers with memo (e.g. THORChain LP), include both memo AND transfer for accurate CU estimation | ||
| // For pure SOL transfers, pass no instructions to get 0 count (avoids blind signing) | ||
| const instructions = contractAddress | ||
| ? await adapter.buildEstimationInstructions({ | ||
| from: account, | ||
| to, | ||
| tokenId: contractAddress, | ||
| value, | ||
| }) | ||
| : memoInstruction | ||
| ? [ | ||
| memoInstruction, | ||
| SystemProgram.transfer({ | ||
| fromPubkey: new PublicKey(account), | ||
| toPubkey: new PublicKey(to), | ||
| lamports: Number(value), | ||
| }), | ||
| ] | ||
| : undefined |
Contributor
There was a problem hiding this comment.
Include the memo instruction in SPL fee estimation.
Line 129 only attaches memoInstruction in the native SOL branch. When contractAddress is set, getFeeData() estimates compute units without the memo, but Line 431 still injects that memo into the real transaction. That can leave computeUnitLimit too low for memo-bearing SPL sends and turn a successful estimate into a failed broadcast.
Suggested fix
- const instructions = contractAddress
- ? await adapter.buildEstimationInstructions({
- from: account,
- to,
- tokenId: contractAddress,
- value,
- })
- : memoInstruction
- ? [
- memoInstruction,
- SystemProgram.transfer({
- fromPubkey: new PublicKey(account),
- toPubkey: new PublicKey(to),
- lamports: Number(value),
- }),
- ]
- : undefined
+ const baseInstructions = contractAddress
+ ? await adapter.buildEstimationInstructions({
+ from: account,
+ to,
+ tokenId: contractAddress,
+ value,
+ })
+ : memoInstruction
+ ? [
+ SystemProgram.transfer({
+ fromPubkey: new PublicKey(account),
+ toPubkey: new PublicKey(to),
+ lamports: Number(value),
+ }),
+ ]
+ : undefined
+
+ const instructions = memoInstruction
+ ? [memoInstruction, ...(baseInstructions ?? [])]
+ : baseInstructionsAlso applies to: 424-431
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Modals/Send/utils.ts` around lines 118 - 145, The SPL-send fee
estimation path omits memoInstruction when contractAddress is set, so
adapter.buildEstimationInstructions(...) produces estimates that miss the memo
and underestimates compute units; to fix, after obtaining the instructions from
adapter.buildEstimationInstructions({ from: account, to, tokenId:
contractAddress, value }), if memoInstruction is defined append it to that
returned instruction array (or pass it via any supported extraInstructions
parameter) so the estimation includes the memo; ensure you handle the case where
buildEstimationInstructions returns undefined and preserve the previous behavior
for pure SOL sends (memo-only branch).
Comment on lines
+442
to
+444
| if (!result?.signatures) { | ||
| throw new Error('Failed to sign Bebop Solana transaction') | ||
| } |
Contributor
There was a problem hiding this comment.
Validate non-empty signatures, not just property existence.
Line 442 currently accepts signatures: [], which can advance with an unusable signed payload.
💡 Suggested fix
- if (!result?.signatures) {
+ if (!result?.signatures?.length) {
throw new Error('Failed to sign Bebop Solana transaction')
}📝 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.
Suggested change
| if (!result?.signatures) { | |
| throw new Error('Failed to sign Bebop Solana transaction') | |
| } | |
| if (!result?.signatures?.length) { | |
| throw new Error('Failed to sign Bebop Solana transaction') | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx`
around lines 442 - 444, The current check in useTradeExecution (variable result
and its signatures property) only verifies existence and misses empty arrays;
change the guard to ensure result.signatures is an array with at least one
element (and optionally contains non-null entries) before proceeding—if not,
throw the same error and prevent advancing with an empty/invalid signed payload;
update the check around the result handling in useTradeExecution to validate
Array.isArray(result.signatures) && result.signatures.length > 0 (and filter out
falsy signatures if you later iterate them).
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.
Does what it says on the box.
merged_untaggedcase was yolo-creating a private sync PR without checking if one already existed - unliketagged_private_stalewhich had the guard. This is what caused #12158 to be created right after #12157 was already merged.Risk
Low - release script only, no prod code touched.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests