Table of Contents
- Table of Contents
- Summary
- Scope
- System Overview
- Solana Chain Adapter
- Security Model and Trust Assumptions
- Privileged Roles
- High Severity
- Medium Severity
- Low Severity
- Mishandling of Exclusivity Deadline Greater Than 1 Year
- Missing SpeedUpV3Deposit and FillV3RelayWithUpdatedDeposit Functionalities in SVM Spoke Pool
- Pausing Mechanism May Be Slow in Emergency Scenarios
- Inconsistent Route Enabling Validation
- Inefficient PDA Derivation in handle_v3_across_message
- Unnecessary Use of InitSpace Macro
- Lack of Event Emission
- Events Emitted Without State Change
- instruction_params Account Remains Open After Execution
- Notes & Additional Information
- Conclusion
Summary
- Type
- DeFi
- Timeline
- From 2024-11-26
- To 2024-12-18
- Languages
- Rust & Solidity
- Total Issues
- 19 (10 resolved, 3 partially resolved)
- Critical Severity Issues
- 0 (0 resolved)
- High Severity Issues
- 2 (0 resolved, 1 partially resolved)
- Medium Severity Issues
- 1 (0 resolved)
- Low Severity Issues
- 9 (3 resolved, 2 partially resolved)
- Notes & Additional Information
- 7 (7 resolved)
Scope
We audited the across-protocol/contracts repository at commit 401e24c.
In scope were the following files:
contracts
├── chain-adapters
│ └── Solana_Adapter.sol
programs
├── multicall-handler
│ └── src
│ └── lib.rs
└── svm-spoke
└── src
├── common
│ ├── mod.rs
│ └── v3_relay_data.rs
├── constants.rs
├── constraints.rs
├── error.rs
├── event.rs
├── instructions
│ ├── admin.rs
│ ├── bundle.rs
│ ├── create_token_accounts.rs
│ ├── deposit.rs
│ ├── fill.rs
│ ├── handle_receive_message.rs
│ ├── instruction_params.rs
│ ├── mod.rs
│ ├── refund_claims.rs
│ ├── slow_fill.rs
│ └── token_bridge.rs
├── lib.rs
├── state
│ ├── fill.rs
│ ├── instruction_params.rs
│ ├── mod.rs
│ ├── refund_account.rs
│ ├── root_bundle.rs
│ ├── route.rs
│ ├── state.rs
│ └── transfer_liability.rs
└── utils
├── bitmap_utils.rs
├── cctp_utils.rs
├── deposit_utils.rs
├── merkle_proof_utils.rs
├── message_utils.rs
├── mod.rs
└── transfer_utils.rs
Additionally, we reviewed the changes introduced in these PRs: - PR 853 at commit 25fc095 - PR 791 at commit 5c49410 - PR 810 at commit 739fb03
System Overview
The SVM Spoke Pool extends Across Protocol's cross-chain bridge network to the Solana ecosystem. The program serves as a functional re-implementation of SpokePool.sol
tailored for the Solana blockchain, incorporating the necessary adaptations for Solana compatibility. It utilizes Circle's Cross-Chain Transfer Protocol (CCTP) to facilitate token and message bridging to and from the Ethereum mainnet. Similar to its EVM-based counterparts, this spoke pool operates under the direction of the EVM hub pool, which manages tasks such as pool rebalancing and relayer repayment.
While maintaining feature parity with EVM Spoke Pools, the Solana implementation addresses fundamental architectural differences:
Account Creation and Lifecycle
- Requires explicit account creation and rent reservation for all storage.
- Implements account closing patterns to return rent when data is no longer needed.
Event Handling
- Leverages
emit_cpi!
macro instead of standardemit!
to overcome Solana's log size limitations and truncation issues. - Ensures complete event data capture for cross-chain communication.
Account Validation through PDAs
- Uses PDAs for vault ownership and state management.
- Uses PDA seeds for secure account validation.
- Enforces authority relationships through PDA derivation paths.
Token Account Management
- Manages tokens through Associated Token Accounts (ATAs) following Solana patterns.
- Creates ATAs for recipients during fills when needed.
Solana Chain Adapter
The Solana Chain Adapter bridges the semantic gap between the EVM and Solana environments. It transforms EVM events and messages into Solana instruction data, manages CCTP message encoding/decoding, and handles cross-chain identifier conversion between Ethereum and Solana address formats.
For relayers and data workers, the adapter provides intuitive interfaces that abstract away the complexities of Solana's programming model. This abstraction ensures that existing Across Protocol participants can seamlessly integrate Solana operations into their workflows.
The adapter's design accommodates Solana's unique characteristics while maintaining protocol consistency. More specifically, it handles:
- Event translation and message formatting
- Cross-chain address transformations
- Instruction data serialization
- Account management abstractions
- PDA derivation and validation
Security Model and Trust Assumptions
The SVM Spoke Pool builds upon Across Protocol's established security foundations while adapting to Solana's security paradigms. CCTP Integration serves as the backbone for cross-chain communication. The implementation relies on Circle's message attestation framework to validate cross-chain messages and facilitate token transfers. This relationship creates a dependency on CCTP's security model, specifically on its message transmitter program and attestation verification mechanisms.
Account authority flows through carefully designed PDA hierarchies. The state PDA acts as the central authority for vault management, while cross-domain admin controls arrive via verified CCTP messages from Ethereum. This structure ensures that only authorized entities can initiate state changes or manage funds. Program validation takes advantage of Anchor's constraint system to enforce account relationships and access controls.
Bundle verification maintains consistency with the broader Across Protocol. Merkle proofs validate relayer refunds and slow fills, with bundle execution orchestrated by the Hub Pool on Ethereum. The implementation preserves state consistency throughout bundle processing while adapting to Solana's account model.
Privileged Roles
The SVM Spoke Pool implements a clear privilege hierarchy:
Cross-Domain Admin (The HubPool on Ethereum mainnet):
- Pauses deposits and fills.
- Manages enabled routes.
- Sets cross-domain admin.
- Relays root bundles for execution.
- Emergency-deletes root bundles.
State Owner (Multisig on Solana):
- Executes all actions available to the Cross-Domain Admin.
- Transfers ownership of the spoke pool.
High Severity
Forced Use of Claim Accounts for All Relayers in Certain Scenarios
When a relayer within a transaction leaf cannot receive direct refunds, such as due to a failed token transfer caused by blacklisting, the protocol uses claim account PDAs to handle the refund. However, this fallback mechanism imposes limitations on other relayers within the same transaction, forcing all of them to claim refunds via a ClaimAccount
PDA, even if direct refunds are possible for them. This leads to the following issues:
- Risk of Hitting Compute Budget: The
accrue_relayer_refunds
process involvesfind_program_address
, which can be computationally expensive as it iteratively attempts to validate a valid bump. Since this process can be used for several accounts, there is a high likelihood that the transaction will revert. Consequently, relayers and users may not be able to retrieve their refunds from that leaf. - Refund Delays: A relayer might claim their refund from its
ClaimAccount
PDA right before the execution of a leaf, reducing the vault’s total available funds. This could delay refunds for other users as the protocol might lack sufficient liquidity at the time of execution. - Increased Execution Overhead: Executors are responsible for ensuring that all relayers have initialized claim accounts and are incurring additional computational and financial overhead, including the cost of covering rent exemption fees.
- Liquidity-Related Reverts: Relayers can attempt to claim refunds when the spoke pool lacks sufficient liquidity. This scenario might result in transaction reversion without providing a meaningful error message, as the hub pool does not track outstanding spoke pool debts.
- Prone to misuse: Since the function can be invoked by anyone, malicious actors could front-run the data worker, unnecessarily deferring refunds even when direct refund mechanisms are operational. In addition, a malicious relayer might force the use of claim accounts and delay its refund claim until just before the execution of a leaf, deliberately triggering a revert to disrupt system operations.
Consider implementing logic to route refunds only through ClaimAccount
PDAs for relayers that are explicitly unable to receive direct transfers. This would optimize operations, prevent errors, and reduce overhead. Moreover, consider enhancing error messaging to clearly indicate insufficient liquidity in the spoke pool during claim attempts. This change would ensure smoother operations and improved clarity for users.
Update: Partially resolved in pull request #847. The team stated:
We acknowledge the issue and have also considered it extensively when originally working on Solana Spoke program development, but have opted not to introduce suggested changes to full extent due to following reasons: - Risk of Hitting Compute Budget: even when not deferring the refunds, also the
distribute_relayer_refunds
method internally callsassociated_token::get_associated_token_address_with_program_id
that involves callingfind_program_address
to check if valid recipient ATA was provided, so there should not be significant additional cost inaccrue_relayer_refunds
logic compared todistribute_relayer_refunds
. We have also tested that this can handle up to 28 relayer refund distributions (both to token accounts and claim accounts) that is above the currentMAX_POOL_REBALANCE_LEAF_SIZE
of 25. Even when hitting this limit, the bottleneck was inner instruction size limit when emittingExecutedRelayerRefundRoot
event, not the compute budget. This is further confirmed by empirical evidence that each iteration infind_program_address
consumes additional 1500 compute units, and given the statistical distribution of canonical bump distribution (99.9% take max 10 iterations), even if all 25 refunds were 10 iterations deep (highly unlikely) that would consume 375'000 CU which is still below maximum allowed 1'400'000 limit. - Refund Delays: We don't see how a relayer claiming its deferred refund can negatively affect spoke pool liquidity for the upcomingexecute_relayer_refund_leaf
call compared to scenario if it had received its refund immediately. The proposed pool rebalance within the root bundle should ensure that a spoke should have sufficient liquidity to service relayer refunds, slow fills, cancelled deposits and repayments to HubPool. The only viable scenario where a spoke can have insufficient liquidity is when rebalance tokens are received later than receiving the bridgedrelay_root_bundle
call, but that should not be further worsened if relayer refunds get deferred due to some token accounts being blocked. The protocol is designed to be resilient to insufficient liquidity on executing relayer refunds and this happens often on EVM chains due to token bridge and message bridge timing being off. It simply delays how long it takes to refund relayers until the tokens arrive at the target spoke pool. - Increased Execution Overhead: claim account initialization can be automated and the rent exemption fees are recovered when claiming the refund. The proposed fix also optimizes refund claim related instructions by moving parameters to accounts so to benefit from compression of transaction compilation (accounts in multiple instructions might use index references to the same account). This allows claiming up to 21 refunds in one transaction when batching the claim instructions. - Liquidity-Related Reverts: We don't see the necessity to introduce additional vault balance checks when claiming accrued refunds as the defaultTokenError::InsufficientFunds
error from the token program should be sufficiently clear. - Prone to misuse: We acknowledge that in the worst case the data worker can fall back to initializing claim accounts, executing deferred refunds and claiming on behalf of all relayers that can receive the refunds to their token accounts. This can further be mitigated by specification in UMIP that refunds have to be sorted by value in the leafs, so that the data worker can opt not to execute leaves for low value refunds that hold larger risk of griefing attacks. - Consider implementing logic to route refunds only throughClaimAccount
PDAs for relayers that are explicitly unable to receive direct transfers: We originally attempted to implement such selective logic, but the lack of try-catch pattern on transfer CPIs in Solana did not allow to do this efficiently. As an alternative, we considered to duplicate the same checks as in token program transfer instruction to catch a case where a refund to token account might get blocked, but viewed that as introducing too much overhead that could be even further complicated when dealing with Token-2022 program extensions that might block a transfer. Considering above we have opted to apply only a limited fix by optimizing the way how relayer refund claim instructions are composed by moving its parameters to account data that should reduce execution overhead when claiming relayer refunds.
Exploitation of Cost Asymmetries Across Chains to Impose High Gas Costs on Refund Execution
The Across protocol is a cross-chain bridge designed for fast and efficient bridging of ERC-20 tokens between supported origin and destination chains. Users, referred to as depositors, initiate the process by depositing their tokens into the protocol on the origin chain. Relayers then fulfill these deposits on the destination chain using their own funds, delivering the specified amount after deducting fees. To reimburse relayers, the protocol either utilizes the available funds on a chain of the relayer's choice or accesses the HubPool
on Ethereum if sufficient liquidity is not available on a specific chain.
However, a malicious user can exploit cost asymmetries between chains to cheaply create a large number of refund leaves that must be executed on a high-cost chain (e.g., Ethereum), imposing a significant financial burden on whoever executes these leaves (often the dataworker which is a public good).
Detailed Attack Steps:
- Spam Low-Cost Deposits: The attacker repeatedly makes minimal-value deposits on a low-cost chain (e.g., Solana or a cheap L2) where transaction fees are very low. Each deposit sets the attacker as the exclusive relayer, ensuring only they can fill that deposit within the exclusivity period.
- Cheap Fills on Another Cheap Chain: The idea is that the target chain is also low-cost. Since the attacker is the exclusive relayer, they can fill their own deposits. This filling process is also inexpensive.
- Specifying Refund on a High-Cost Chain: When the attacker fills these deposits, they specify their refund to be delivered on a high-cost chain (e.g., Ethereum mainnet).
- Multiple Addresses: The attacker can include multiple refund addresses in these fills. The number of addresses requires the creation of a large number of leaves, each containing about 25 refund addresses.
The attacker’s end result: a large number of leaves, each containing numerous refunds (a mix of negligible and real refunds) that are costly to execute on Ethereum. Executing a relayer refund leaf is resource-intensive as it involves verifying Merkle proofs, performing multiple token transfers, and running various validations.
The dataworker or any other executor of these leaves faces a lose-lose scenario:
- Execute the leaves and incur high gas costs, which is unsustainable.
- Leave the refunds unexecuted, causing legitimate (but unfortunately batched) refunds within these leaves to remain unpaid.
- In this scenario, legitimate relayers (who want their refunds to be paid on high-cost chains) and users (including those whose deposits have expired) who share leaves with these spam entries cannot access their rightful refunds unless someone pays the execution costs on Ethereum.
Beyond this direct on-chain execution cost, the dataworker must also handle extensive off-chain tasks—such as building large Merkle trees, managing and storing more event data, and potentially writing to a data availability layer. All of this adds substantial operational costs for the dataworker. Moreover, a high volume of tiny, spammy deposits clutters the protocol for everyone. Relayers, block explorers, and other third parties that rely on parsing the events now face more difficulty separating genuine activity from noise.
To address this issue, consider implementing measures such as the following:
- Require a minimum deposit value on low-cost chains to reduce the feasibility of large-scale spamming, thereby ensuring that only economically significant transactions are processed.
- Update the UMIP to ignore or filter out spammy transactions, ensuring that only legitimate transactions are eligible for execution.
Medium Severity
Overuse of emit_cpi!
Macro for Event Logging
The emit_cpi!
macro is extensively utilized across the codebase for logging events, even in scenarios where the standard emit!
macro would suffice. While emit_cpi!
provides benefits such as preventing log truncation by leveraging Solana’s instruction data storage, its overuse introduces unnecessary computational overhead.
The emit_cpi!
macro works by executing a self-invocation with the event data, ensuring logs remain intact regardless of size. However, this approach has trade-offs, as creating and invoking new instructions consumes additional compute budget within transactions. Solana’s transactions have strict compute budget limits, and excessive use of emit_cpi!
can result in these limits being exceeded, potentially impacting the program's functionality.
To mitigate these issues, consider limiting the use of emit_cpi!
to the following cases:
- Events that are likely to exceed the log size limit (10 KB) and are critical to preserve.
- Events that might be subject to truncation in malicious scenarios.
- Events emitted alongside other logs where truncation is likely.
For non-sensitive events or those occurring behind admin actions in which goodwill is assumed, the emit!
macro is more appropriate. Doing this ensures that compute resources are utilized efficiently without compromising the reliability of critical logs.
Update: Acknowledged, not resolved. The team stated:
We have decided not to replace some of the emit_cpi! events with native emit! events. This decision stems from the potential risk of an attacker prepending instructions that fill the logs before invoking the svm-spoke functions, which could cause the logs to be dropped. To mitigate this risk, we opted to continue using emit_cpi!. While certain cases, might be safe to use native emit! events, we chose to maintain emit_cpi! for consistency across all events. Additionally, it is preferable to always query events using the same technique in all contexts, not requiring some code to use emit_cpi! and others to use emit! logic.
Low Severity
Mishandling of Exclusivity Deadline Greater Than 1 Year
An exclusivity_parameter
can be passed as an argument to deposits, which specifies a window where only an exclusive relayer will be able to fill the order. If the parameter is less than the max exclusivity period (currently set to 1 year), then the specified time window is added to the current time. If the window is set to greater than 1 year, the exclusivity deadline is set to the provided value. In case this value is less than the current time, it will have the same effect as with no exclusivity parameter set.
Consider handling this with an error to avoid scenarios where a user thinks that they are setting a large deadline for exclusivity but are, in effect, allowing any relayer to fill their order.
Update: Acknowledged, not resolved. The team stated:
We have decided to acknowledge the issue but not implement the proposed change. We want to mimic the behavior in the EVM SpokePool, where we do not revert when the exclusivity_parameter is less than the current time and more than 1 year, resulting in a no-exclusivity parameter setting. Blocking this configuration was deemed unnecessary at the contract level. For reference, this is where the exclusivityParameter was proposed in EVM: https://github.com/across-protocol/contracts/pull/670. Instead, the UI will display warnings and prevent users from submitting this type of misconfiguration in the dApp. It’s worth noting that this issue only affects depositors intending to set exclusivity periods longer than one year, which is not a valid or intended use of the protocol. As such, not permitting values of this size has no impact on any users.
Missing SpeedUpV3Deposit
and FillV3RelayWithUpdatedDeposit
Functionalities in SVM Spoke Pool
The speedUpV3Deposit
function, implemented in EVM Spoke Pools, allows users to signal updated output amounts, recipient addresses, and messages to the relayer. However, this functionality is not present in the SVM Spoke Pool. Similarly, the fillV3RelayWithUpdatedDeposit
function is also absent from the SVM Spoke Pool which is an alternative to the fillV3Relay
function for filling deposits. The absence of these two functions creates functional inconsistencies between the EVM and SVM Spoke Pools. As such, this inconsistency could confuse users who expect uniform functionality across platforms and can reduce operational flexibility in managing deposits and relays.
Consider implementing both speedUpV3Deposit
and fillV3RelayWithUpdatedDeposit
functionalities in the SVM Spoke Pool to align with the EVM Spoke Pools. Alternatively, clearly document their absence to manage user expectations effectively.
Update: Partially resolved in pull request #848. The team stated:
We acknowledge this issue and will address it by adding a comment in the code to explain why we decided to drop support for speed-ups on Solana, given the following limitations: Speed-ups are not used often, especially with deposit expirations in place. Previously, they were used to update deposit parameters, but expirations now handle this effectively. Supporting speed-ups on Solana is overly complex due to incompatible cryptographic schemes. Solana wallets (Ed25519) and Ethereum (ECDSA secp256k1) use different signature algorithms: While ECDSA signatures from Ethereum wallets can be verified on Solana, Solana wallets cannot generate ECDSA signatures that can be verified on Ethereum. Speed-ups require the depositor’s signature to be validated on both source and destination chains, which demands a shared signature scheme—something not feasible between Solana and Ethereum.
Pausing Mechanism May Be Slow in Emergency Scenarios
The SVM Spoke pool enables both the local owner and the remote HubPool to pause deposits and fills independently, which enhances flexibility in granular pausing operations. However, the current design may not be ideal during emergencies. The local owner is going to be controlled by a multisig, while the hubpool requires the execution of relaySpokePoolAdminFunction
, which also depends on a multisig. These multisig dependencies can introduce delays when rapid pausing is required in critical scenarios.
Consider adopting a more efficient pausing mechanism, such as introducing a dedicated PAUSER_ROLE
assigned to a system account. This role can enable quick reactions to emergencies. To balance speed and security, consider doing the following:
- Designate the contract owner as a backup account with pausing capabilities to maintain redundancy.
- Separate the pausing role from the owner role to follow the principle of least privilege and reduce abuse risks.
- Ensure that the pausing role is revocable by the owner to allow adaptation to operational needs.
Update: Acknowledged, not resolved. The team stated:
We acknowledge the issue/remark and appreciate your input. However, in line with our efforts to closely mimic the protocol design on EVM chains, we believe that introducing an admin for the PAUSER_ROLE is neither necessary nor justified, as it would introduce a deviation from the design of the EVM SpokePool. In emergency scenarios, we are confident that the combined authority of the cross-chain owner and local owner provides sufficient mechanisms to respond promptly and effectively. Therefore, we have decided not to implement the suggested mechanism at this time.
Inconsistent Route Enabling Validation
The SetEnableRoute
function allows enabling or disabling a route for deposits from an origin token to a destination chain ID. This function can be called by two distinct roles: the owner and the remote owner (the HubPool on mainnet using CCTP).
Currently, there is a validation present in the Solana_Adapter
that restricts the remote owner from enabling a route if the origin token is different from the USDC address on Solana. However, this validation is not present when the local owner calls the function directly on the svm-spoke
program. This discrepancy can lead to inconsistencies and introduce the potential for incorrect tokens to be enabled inadvertently.
Consider implementing the same validation for the local owner to ensure uniformity across roles.
Update: Acknowledged, not resolved. The team stated:
We accept this issue as expected behavior because, in the EVM Solana_Adapter, enforcing the use of USDC is necessary to map the EVM-truncated Solana USDC address to its corresponding bytes32 address. However, this requirement does not apply on the Solana side, as the local owner (multisig) can directly provide the full origin token address as needed. The design as currently implemented also enables the SVM spoke to support tokens beyond USDC in the future without needing to change the spoke, and simply changing the EVM Solana_Adapter to store an additional mapping. Hardcoding and validation on the SVM side would force new tokens to a SVM spoke upgrade.
Inefficient PDA Derivation in handle_v3_across_message
The handle_v3_across_message
function within multicall_handler
relies on find_program_address
to derive the public key of a PDA and its canonical bump value, using a seed and the program ID. The find_program_address
function computes a valid PDA by calling create_program_address
iteratively, starting with a bump of 255 and decrementing the bump until a valid address is found. While this ensures that a valid PDA is derived, the iterative process is computationally expensive, which is problematic, particularly in functions like handle_v3_across_message
that may execute multiple instructions. If not optimized, this approach risks exceeding the compute budget.
To mitigate this issue, consider storing the derived bump value in an account's data field upon initialization. This approach enables subsequent instructions to reuse the stored bump, eliminating the need for repeated calls to find_program_address
.
Update: Partially resolved in pull request #846. The team stated:
We acknowledge the issue, though storing the bump would require passing additional state account with
HandleV3AcrossMessage
. The depositor should then encode the message by prepending this state account to all other accounts required for all inner CPIs, but that would not be compatible with lamports value transfers to the first account when value > 0. To resolve that would require extending theAcrossPlusMessage
struct to distinguish between static accounts that are passed tohandle_v3_across_message
(currently we have none) from all remaining accounts that are used in inner CPIs. This would mean longer message size and more complexity for both the depositors and fillers. As an alternative measure, we propose for the deployer to choose such handler program ID so to minimize the resulting computational cost of finding the PDA for thehandler_signer
making sure that its bump is the maximum value of 255. The linked PR adds a comment on program ID on this consideration.
Unnecessary Use of InitSpace
Macro
Throughout the codebase, the following structs use the InitSpace
macro unnecessarily:
FillV3RelayParams
ExecuteRelayerRefundLeafParams
RequestV3SlowFillParams
ExecuteV3SlowRelayLeafParams
These accounts are initialized using the initialize_instruction_params
method, which creates empty accounts with a size specified in the function's instructions. Consequently, the InitSpace
macro, which implements the Space
trait, becomes redundant as the INIT_SPACE
constant generated by this trait is not utilized. The inclusion of the InitSpace
macro in these cases adds unnecessary overhead and obscures the intended logic, given that its functionality is not employed.
Consider removing the InitSpace
macro from these structs to simplify the codebase and remove superfluous logic. This adjustment would help improve code clarity and align it more closely with actual use patterns.
Update: Resolved in pull request #839.
Lack of Event Emission
The transfer_ownership
function does not emit any relevant events after performing sensitive actions.
Consider emitting events after sensitive changes take place to facilitate off-chain tracking and monitoring.
Update: Resolved in pull request #838.
Events Emitted Without State Change
The redundant emission of events can mislead observers monitoring events to track state changes, causing them to process events that do not signify actual modifications. This reduces the reliability of events as a source of truth for state transitions.
Throughout the codebase, multiple instances of events being emitted without any state change were identified:
-
pause_deposits
andpause_fills
functions emit thePausedDeposits
andPausedFills
events, respectively, regardless of whether the pause state is altered. -
The
set_cross_domain_admin
function emits theSetXDomainAdmin
event every time it is called, even when theHubPool
remains unchanged. -
The
set_enable_route
function emits theEnabledDepositRoute
event even if theenabled
value for a route does not differ from its previous state.
Consider updating these functions to check if the new state differs from the current state before emitting their respective events. This adjustment would ensure that only meaningful state changes trigger events, improving event reliability and reducing unnecessary processing for observers.
Update: Acknowledged, not resolved. The team stated:
We acknowledge the issue, but we’ve decided not to make changes for the following reasons: In the EVM, we follow a similar pattern where events are emitted irrespective of the previous state. Sticking to this approach ensures consistency across our implementations and keeps the code simpler. By always emitting events, we provide observers with a clear log of all function calls, even when no state changes occur, which can be valuable for debugging and auditing purposes.
instruction_params
Account Remains Open After Execution
The instruction_params
account used in the ExecuteRelayerRefundLeaf
instruction remains open after execution, despite becoming unnecessary once the instruction completes. Leaving the account open leads to resource inefficiencies. Although a close_instruction_params
function can address this issue, it introduces unnecessary complexity. A more elegant and efficient approach is to leverage Anchor's close
attribute to handle account closure automatically. Unclosed accounts within Solana persist on-chain indefinitely, wasting storage resources and keeping the lamports from rent locked and unavailable for other operations.
Consider using Anchor's close
attribute to ensure that the instruction_params
account is automatically closed, reclaiming resources and simplifying the code.
Update: Resolved in pull request #840.
Notes & Additional Information
Incomplete or Misleading Comments
Throughout the codebase, multiple instances of incomplete or misleading comments were identified:
- This comment above
unsafe_deposit_v3
states that thedeposit_nonce
is not used to derive thedeposit_id
, whereas it is so used. - This comment above
execute_relayer_refund_leaf
states that it can only be called by the owner, whereas it can be called by anyone. For context,relay_root_bundle
is the admin function where the admin provides the latest root bundles and increments the bundle ID within the program state. - This comment above
execute_relayer_refund_leaf
states that the structure and validation of the leaf are defined in the UMIP, but there is no link or number of the UMIP.
Update: Resolved in pull request #843.
Unused Errors
Throughout the codebase, multiple instances of unused errors were identified:
Consider removing any unused errors.
Update: Resolved in pull request #841.
Duplicate Logic
The claim_relayer_refund
and claim_relayer_refund_for
functions share almost identical logic, with the primary distinction being the refund address. This duplication introduces maintainability challenges, as future updates may need to be applied to both functions simultaneously, increasing the risk of inconsistencies.
To streamline the code and reduce redundancy, consider implementing the following refactor:
- Abstract the common logic from both functions into a helper function that handles the transfer and event emission. This helper could accept parameters such as the target token account and any additional context-specific data.
- Alternatively, consolidate the instructions into a single function that takes an optional
refund_address
parameter, enabling flexible use cases while maintaining a single code path.
Implementing either of these approaches would simplify future maintenance and reduce the surface area for potential bugs.
Update: Resolved in pull request #845.
Unneccessary PDA Signer Can Be Used in Multicall Handler
Across deposits may include additional message data that must be processed during relayer fills, in which case the data is deserialized and processed as various accounts and instructions that are invoked on the message handler specified in the relayer data. In the provided multicall_handler
example, the handler_signer
may be included as an additional signer in the CPI calls decoded from the message. However, it may be included when not necessary. This is because, in each program call, use_handler_signer
is set if any accounts in the call match the handler_signer
key, but it is never reset to false
before checking subsequent calls.
Consider correcting this logic by resetting the value of use_handler_signer
at the top of the outermost for
loop to avoid passing additional unnecessary signers in CPI calls.
Update: Resolved in pull request #837.
Ambiguous Parameter Usage for fill_deadline
The deposit_v3
and deposit_v3_now
functions use the same parameter name, fill_deadline
, with differing semantics. In deposit_v3
, it is an absolute timestamp, whereas in deposit_v3_now
, it is an offset added to the current time.
The documentation lacks clarity about this distinction, increasing the likelihood of errors. Users might mistakenly pass an absolute timestamp to deposit_v3_now
, leading to a computed deadline (current_time + fill_deadline_offset
) that exceeds current_time + state.fill_deadline_buffer
. This results in failed transactions.
Consider renaming fill_deadline
in deposit_v3_now
to fill_deadline_offset
for clarity. Alternatively, consider improving the documentation to explicitly state the difference in semantics, including examples.
Update: Resolved in pull request #844.
Unnecessary Use of Instruction
Attribute
The Accounts
derive macro includes an attribute named Instruction
that provides access to the instruction’s arguments within the struct utilizing the macro. However, in the WriteInstructionParamsFragment
struct, this attribute is unnecessary, as neither the offset
nor the fragment
arguments are used within the struct.
This inclusion may lead to confusion for developers working on the codebase. As such, consider removing the unused attribute to improve code clarity and maintainability.
Update: Resolved in pull request #842.
TODO Comments in the Code
During development, having well-described TODO comments will make the process of tracking and resolving them easier. Without this information, these comments might age and important information for the security of the system might be forgotten by the time it is released to production.
Throughout the codebase, multiple instances of unaddressed TODO comments were identified:
Consider removing all instances of TODO comments and instead tracking them in the issues backlog. Alternatively, consider linking each inline TODO to a corresponding backlog issue.
Update: Resolved in pull request #815.
Conclusion
The SVM Spoke Pool represents a significant extension of the Across Protocol to the Solana ecosystem, re-implementing the core spoke pool functionality while adapting to Solana's unique programming model. Through CCTP integration, it enables secure token bridging and message passing between Solana and Ethereum, maintaining feature parity with EVM spoke pools while embracing Solana's native patterns for account management and program interaction.
During our audit, we found the codebase to be thoughtfully structured with careful consideration for Solana's security boundaries and programming constraints. The implementation demonstrates a strong understanding of both Solana's account model and Across Protocol's architectural requirements. While we identified some issues requiring attention, particularly around the execution of the relayer refund leaves, the overall implementation is robust and well-architected.
Throughout the engagement, the Risk Labs team remained consistently helpful, promptly answering all our questions and thoroughly explaining the protocol's details.