| sponsor | slug | date | title | findings | contest |
|---|---|---|---|---|---|
Superposition |
2024-10-superposition |
2025-01-07 |
Superposition |
449 |
Code4rena (C4) is an open organization consisting of security researchers, auditors, developers, and individuals with domain expertise in smart contracts.
A C4 audit is an event in which community participants, referred to as Wardens, review, audit, or analyze smart contract logic in exchange for a bounty provided by sponsoring projects.
During the audit outlined in this document, C4 conducted an analysis of the Superposition smart contract system written in Solidity and Rust. The audit took place between October 18 — November 1, 2024.
4 Wardens contributed reports to Superposition:
This audit was judged by 0xsomeone.
Final report assembled by thebrittfactor.
The C4 analysis yielded an aggregated total of 6 unique vulnerabilities. Of these vulnerabilities, 3 received a risk rating in the category of HIGH severity and 3 received a risk rating in the category of MEDIUM severity.
Additionally, C4 analysis included 3 reports detailing issues with a risk rating of LOW severity or non-critical.
All of the issues presented here are linked back to their original finding.
The code under review can be found within the C4 Superposition repository, and is composed of 21 smart contracts written in the Solidity programming language and includes 4273 lines of Solidity code.
C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.
High-level considerations for vulnerabilities span the following key areas when conducting assessments:
- Malicious Input Handling
- Escalation of privileges
- Arithmetic
- Gas use
For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.
Submitted by ZanyBonzy
SeaWaterAMM.sol holds the createPoolD650E2D0 which allows the admin to initialize a new pool. It calls the create_pool_D650_E2_D0 function in the stylus.
As can be seen from the create_pool_D650_E2_D0 function, it takes in the token address, sqrtPriceX96 and fee.
pub fn create_pool_D650_E2_D0(
&mut self,
pool: Address,
price: U256,
fee: u32,
) -> Result<(), Revert> {
//...
}
}But createPoolD650E2D0's definition takens in more, token address, sqrtPriceX96, fee, tick spacing and maxLiquidityPerTick, causing a mismatch between the function definitions of the Solidity and Stylus contracts.
function createPoolD650E2D0( //@audit
address /* token */,
uint256 /* sqrtPriceX96 */,
uint32 /* fee */,
uint8 /* tickSpacing */,
uint128 /* maxLiquidityPerTick */
) external {
directDelegate(_getExecutorAdmin());
}Calls to the function will always fail, breaking SeawaterAMM.sol's functionality to create a pool position.
Remove the unneeded parameters.
function createPoolD650E2D0( //@audit
address /* token */,
uint256 /* sqrtPriceX96 */,
uint32 /* fee */,
- uint8 /* tickSpacing */,
- uint128 /* maxLiquidityPerTick */
) external {
directDelegate(_getExecutorAdmin());
}Context
af-afk (Superposition) confirmed
0xsomeone (judge) increased severity to High and commented:
The Warden has correctly identified that the function definitions of the Solidity and Stylus contracts differ, resulting in the relevant functionality of the system being inaccessible.
In line with the previous audit's rulings, I believe a high-risk rating is appropriate for this submission as pool creations are rendered inaccessible via any other functions in contrast to the original audit's submission which permitted circumvention of this error.
I believe some key pieces of information are missing to provide an accurate severity assessment, which I will address in this comment.
It is true that
createPoolD650E2D0will not work if directly called, as it has the wrong ABI, and this finding is technically valid. However, there is a fallback function that allows the creation of new pools by using the correct ABI.The correct ABI, like this issue points, is the following:
createPoolD650E2D0(address,uint256,uint32)So the third byte is
0x80:function testAbi() public pure returns (bytes1) { return abi.encodeWithSignature("createPoolD650E2D0(address,uint256,uint32)", address(0), 0, 0)[2]; }decoded output { "0": "bytes1: 0x80" }
If we look at the fallback function, the execution will fall under the executor fallback:
fallback() external { // swaps if (uint8(msg.data[2]) == EXECUTOR_SWAP_DISPATCH) directDelegate(_getExecutorSwap()); // update positions else if (uint8(msg.data[2]) == EXECUTOR_UPDATE_POSITION_DISPATCH) directDelegate(_getExecutorUpdatePosition()); // positions else if (uint8(msg.data[2]) == EXECUTOR_POSITION_DISPATCH) directDelegate(_getExecutorPosition()); // admin else if (uint8(msg.data[2]) == EXECUTOR_ADMIN_DISPATCH) directDelegate(_getExecutorAdmin()); // swap permit 2 else if (uint8(msg.data[2]) == EXECUTOR_SWAP_PERMIT2_A_DISPATCH) directDelegate(_getExecutorSwapPermit2A()); // quotes else if (uint8(msg.data[2]) == EXECUTOR_QUOTES_DISPATCH) directDelegate(_getExecutorQuote()); else if (uint8(msg.data[2]) == EXECUTOR_ADJUST_POSITION_DISPATCH) directDelegate(_getExecutorAdjustPosition()); else if (uint8(msg.data[2]) == EXECUTOR_SWAP_PERMIT2_B_DISPATCH) directDelegate(_getExecutorSwapPermit2B()); -> else directDelegate(_getExecutorFallback()); }Current values:
uint8 constant EXECUTOR_SWAP_DISPATCH = 0; uint8 constant EXECUTOR_UPDATE_POSITION_DISPATCH = 1; uint8 constant EXECUTOR_POSITION_DISPATCH = 2; uint8 constant EXECUTOR_ADMIN_DISPATCH = 3; uint8 constant EXECUTOR_SWAP_PERMIT2_A_DISPATCH = 4; uint8 constant EXECUTOR_QUOTES_DISPATCH = 5; uint8 constant EXECUTOR_ADJUST_POSITION_DISPATCH = 6; uint8 constant EXECUTOR_SWAP_PERMIT2_B_DISPATCH = 7;Moreover, creating a pool is permissionless and intended by the Sponsor, it doesn't have to be called by the executor admin; the executor fallback would be able to create new pools.
Therefore, there is no loss of funds, and the functionality of the protocol is not impacted in this way; I don't see how a High risk can be justified. I believe this issue falls under the QA umbrella, as a function does not work according to specifications.
@DadeKuma - This submission's assessment is in line with the previous audit and the presence of a fallback mechanism is not sufficient to justify the finding's invalidation. Otherwise, any inaccessible functionality of the system could be argued as being present in the fallback function and all findings pertaining to it would have to be invalidated in the previous audit as well.
Given that wardens were aware of the judgment style of this particular submission type, I do not believe that downgrading it in the follow-up round is a fair approach.
Submitted by ZanyBonzy, also found by Q7, Tigerfrake, and DadeKuma
In swap_2_internal, if the first pool doesn't have enough liquidity, amount_in could be less than original_amount, and as expected, amount_in is taken from swapper. But the function still refunds original_amount - amount_in to the user if original_amount is more than amount_in.
From the function, we can see than amount_in is taken from swapper. Then the function checks if original_amount is more than amount_in, before which the difference is transferred back to the sender.
>> erc20::take(from, amount_in, permit2)?;
erc20::transfer_to_sender(to, amount_out)?;
>> if original_amount > amount_in {
erc20::transfer_to_sender(
to,
original_amount
>> .checked_sub(amount_in)
.ok_or(Error::TransferToSenderSub)?,
)?;
}An unnecessary refund is processed leading to loss of funds for the protocol. Malicious users can take advantage of this to "rob" the protocol of funds through the refunds.
No need to process refunds since amount_in is already taken.
erc20::take(from, amount_in, permit2)?;
erc20::transfer_to_sender(to, amount_out)?;
- if original_amount > amount_in {
- erc20::transfer_to_sender(
- to,
- original_amount
- .checked_sub(amount_in)
- .ok_or(Error::TransferToSenderSub)?,
- )?;
}Context
af-afk (Superposition) confirmed
The submission and its duplicates have correctly identified that the refund process in the
swap_2_internal_erc20function is extraneous and thus results in excess funds being sent to the user.I believe a high-risk severity rating is appropriate as the issue manifests itself in all cases and would result in direct fund loss for the AMM pair.
af-afk (Superposition) commented:
For Issue #12 @0xsomeone how does this compare to your findings here?
@af-afk - I am unsure what comparison is to be drawn here. None of the findings are mine as I am a judge, and I do not believe that the finding referenced has any relation to this one when it comes to impact.
af-afk (Superposition) commented:
Sorry, I should clarify, I mean your assessment that both are valid. It's not possible for both of these to be correct, right? I'm of the opinion that this refund should not be implemented after consideration (and this submission) since the contract's quoting functionality should indicate that this is taking place.
@af-afk - the original submission shared was submitted in a audit that relies on a different commit hash from this one. As we can observe in the highlighted code segment, the code originally transferred the
original_amountfrom thefromaddress.In the remediated code that was part of this audit, the code was updated to simultaneously extract the
amount_infrom the user and perform a refund. The incorrect aspect is that two different solutions for the same problem were incorporated, rendering the refund to be extraneous. I hope this clears things up!
af-afk (Superposition) commented:
Fixed: https://github.com/fluidity-money/long.so/commit/9c7657e8336208e3397b30c32d557379f88a5b87
Submitted by DadeKuma
An attacker can sandwich a user withdrawing funds as there is no way to put slippage protection, which will cause a large loss of funds for the victim.
decr_position_09293696 function was removed entirely. Now, the only way for users to withdraw funds is by calling update_position_C_7_F_1_F_740 with negative delta.
The issue is that in this way, users can't have any slippage protection. decr_position allowed users to choose an amount_0_min and amount_1_min of funds to receive, which is now zero.
This allows an attacker to sandwich their withdrawal to steal a large amount of funds.
Consider reintroducing a withdrawal function that offers slippage protection to users (they should be able to choose amount_0_min, amount_1_min, amount_0_desired, and amount_1_desired).
af-afk (Superposition) acknowledged
The submission has demonstrated that liquidity withdrawals from the system are inherently insecure due to being open to arbitrage opportunities as no slippage is enforced.
I am unsure why the Sponsor has opted to acknowledge this submission as it is a tangible vulnerability and one that merits a high-risk rating. The protocol does not expose a secure way to natively extract funds from it whilst offering this functionality for other types of interactions.
af-afk (Superposition) commented:
@0xsomeone - we won't fix this for now since Superposition has a centralised sequencer, and there's no MEV that's possible for a third-party to extract using the base interaction directly with our provider.
@af-afk - I highly suggest fixing this issue, as a centralized sequencer does not prevent MEV extraction. You can check this impact on Arbitrum, for example.
Submitted by DadeKuma
The admin can't set a pool's protocol fee because the function has not been implemented.
The previous issue, M-12, wasn't fixed properly.
The root cause is that the admin has no way to call set_fee_protocol_C_B_D_3_E_C_35 through Seawater, as the function wasn't implemented.
As a result, the original issue persists because protocol fees cannot be set.
Consider adding the following function to SeaWaterAMM.sol:
function setFeeProtocolCBD3EC35(address /* pool */, uint8 /* feeProtocol0 */, uint8 /* feeProtocol1 */) external {
directDelegate(_getExecutorAdmin());
}Access Control
af-afk (Superposition) commented:
0xsomeone - It's possible to call this function since the signature resolves it to the admin facet in the fallback.
Per discussions in #8 this is a valid, albeit, Medium risk issue.
af-afk (Superposition) commented:
We felt this was technically inaccurate given that the function signature corresponded to the right fallback, triggering the correct dispatch, but we opted to fix this in principal with the similar issues. We weren't responsive at the time to affect the ruling.
Submitted by Tigerfrake, also found by DadeKuma
Both the update_position_internal() and adjust_position_internal() functions are responsible for managing token positions, which involves taking tokens from users. However, there is a critical inconsistency in how each function verifies the operational status of the liquidity pool before performing token transfers from the user.
update_position_internal() - checks if the pool is enabled before taking tokens from the user.
// if we're TAKING, make sure that the pool is enabled.
assert_or!(pool.enabled.get(), Error::PoolDisabled);
---SNIP---
erc20::take(pool_addr, token_0.abs_pos()?, permit_0)?;
erc20::take(FUSDC_ADDR, token_1.abs_pos()?, permit_1)?;adjust_position_internal() - does not explicitly check whether the pool is enabled before proceeding.
let (amount_0, amount_1) =
self.pools
.setter(pool)
.adjust_position(id, amount_0_desired, amount_1_desired)?;
---SNIP---
erc20::take(pool, amount_0, permit_0)?;
erc20::take(FUSDC_ADDR, amount_1, permit_1)?;First, it calls self.pools.setter(pool).adjust_position(...) which has the following comment:
// [update_position] should also ensure that we don't do this on a pool that's not currently running
self.update_position(id, delta)The comment in the adjust_position() function implies that a check for the pool's operational state is necessary and should be enforced in update_position(). However, update_position() function does not make such enforcement as it does not check for pool status.
Users could unintentionally have their tokens adjusted or transferred to a pool that is not operational which is not in accordance with protocol requirement. This also exposes users to risks in the event that there are potential issues with the pool.
Modify the adjust_position_internal() function to include a status check before executing the position adjustment:
// Ensure the pool is enabled before making any adjustments
+ assert_or!(pool.enabled.get(), Error::PoolDisabled);af-afk (Superposition) acknowledged and commented via duplicate Issue #4:
We made the decision that we were going to allow this functionality for now. The reason being that in a programmatic context, the pool can be controlled to be enabled and disabled depending on the broader environment. We use this for example with 9lives to prevent trading of a market that's expired, in lieu of remembering ownership at different time points of the asset. We made the decision that allowing people to supply liquidity could be useful in the future, and for this reason we also allowed supplying liquidity to a frozen pool as well.
0xsomeone (judge) commented via duplicate Issue #4:
The submission claims that an issue submitted in the original audit was not resolved properly; however, the Sponsor's choice to acknowledge the issue does not contradict the original issue's acceptance. As this audit is a follow-up one, it would have been helpful to discuss with the Sponsor directly about their intentions on how to resolve issue #31 of the original audit.
I believe that the issue is invalid based on the Sponsor's intentions.
DadeKuma (warden) commented via duplicate Issue #4:
@0xsomeone - The original issue was fixed, but it introduced another bug (this issue).
Code documentation clearly states that adding liquidity to disabled pools shouldn't be possible:
Requires the pool to be enabled unless removing liquidity. Moreover, it's actually not possible to add liquidity to disabled pools by using the following path, like the documentation suggests:
update_position_C_7_F_1_F_740 > update_position_internalBut it is still possible by using the path described in this issue:
incr_position_E_2437399 > adjust_position_internal > adjust_position > update_positionEven if the documentation states that this shouldn't be possible here:
// [update_position]should also ensure that we don't do this on a pool that's not currently// runningThis is clearly a discrepancy, and I strongly believe this is a valid issue based on the information available during the audit.
0xsomeone (judge) commented via duplicate Issue #4:
@DadeKuma - I believe the discrepancies between the documentation and the implementation are adequate to merit a proper medium-risk vulnerability rating and have re-instated it so.
af-afk (Superposition) commented:
Submitted by Tigerfrake
In the swap_internal() function, the slippage check uses the || operator to validate the swap results. This can lead to a scenario where one of the amounts (amount_0_abs or amount_1_abs) is allowed to be zero, potentially resulting in unwanted slippage.
assert_or!(
amount_0_abs > U256::zero() || amount_1_abs > U256::zero(), // Problematic operator
Error::SwapResultTooLow
);Using the || operator allows the swap to proceed even if one of the amounts is zero, which could lead to unacceptable slippage.
Scenario:
- Consider a user swapping
100units of token A (amount_0) for token B (amount_1). - Due to slippage,
token B’s output (amount_1_abs) becomeszero, while token A’s output (amount_0_abs) remains positive. - With the current
||operator, theswapwould still be consideredvalidsince one amount is greater than zero, even though the user receives no token B (amount_1_abs = 0), resulting in a poor outcome for the user.
Using the || operator means that one token amount can be zero while the other passes the check, leading to an imbalanced swap that might not meet user expectations.
Replace the || operator with the && operator to ensure both token amounts are greater than zero.
assert_or!(
- amount_0_abs > U256::zero() || amount_1_abs > U256::zero(),
+ amount_0_abs > U256::zero() && amount_1_abs > U256::zero(),
Error::SwapResultTooLow
);Invalid Validation
af-afk (Superposition) confirmed
The Warden has identified an incorrect conditional clause that would permit either zero-input non-zero output swaps or non-zero input zero output ones, the latter of which may occur in a realistic scenario and would be unacceptable for the user.
I consider a medium-risk severity rating to be acceptable for this behavior as it would solely manifest in low transaction amounts and high value discrepancy AMM pairs.
af-afk (Superposition) commented:
Fixed: https://github.com/fluidity-money/long.so/commit/b0d39cf8d1be2096cba9c845e424b17c958847c5
For this audit, 3 reports were submitted by wardens detailing low risk and non-critical issues. The report highlighted below by Rhaydden was marked as best from the judge and is included for completeness.
The following wardens also submitted reports: Tigerfrake and rare_one.
StoragePool::init function in pool.rs initializes a new pool but does not set the enabled flag to true. This creates a non-obvious two-step process where pools must be explicitly enabled after initialization before they can be used. All key pool operations (create_position, swap, collect_protocol, collect) check this flag with assert_or!(self.enabled.get(), Error::PoolDisabled) and will revert if the pool is not enabled.
pub fn init(
&mut self,
price: U256,
fee: u32,
tick_spacing: u8,
max_liquidity_per_tick: u128,
) -> Result<(), Revert> {
assert_or!(!self.initialised.get(), Error::PoolAlreadyInitialised);
assert_or!(fee <= 10000, Error::BadFee);
self.initialised.set(true);
self.sqrt_price.set(price);
self.cur_tick
.set(I32::lib(&tick_math::get_tick_at_sqrt_ratio(price)?));
self.fee.set(U32::lib(&fee));
self.tick_spacing.set(U8::lib(&tick_spacing));
self.max_liquidity_per_tick
.set(U128::lib(&max_liquidity_per_tick));
Ok(())
}As a result, there'll be confusion when pools appear initialized but operations fail and also potential delays between pool creation and usability.
If pools should be usable immediately after initialization, consider modifying the init function to set enabled to true:
pub fn init(
&mut self,
price: U256,
fee: u32,
tick_spacing: u8,
max_liquidity_per_tick: u128,
) -> Result<(), Revert> {
assert_or!(!self.initialised.get(), Error::PoolAlreadyInitialised);
assert_or!(fee <= 10000, Error::BadFee);
self.initialised.set(true);
+ self.enabled.set(true); // Enable pool after initialization
self.sqrt_price.set(price);
self.cur_tick
.set(I32::lib(&tick_math::get_tick_at_sqrt_ratio(price)?));
self.fee.set(U32::lib(&fee));
self.tick_spacing.set(U8::lib(&tick_spacing));
self.max_liquidity_per_tick
.set(U128::lib(&max_liquidity_per_tick));
Ok(())
}The grant_position function is to require that a position must not have an owner before granting ownership as hinted here. Albeit, the function does not enforce this requirement. The function directly sets the new owner without verifying if the position is already owned, which could allow unauthorized overwriting of position ownership.
/// Makes the user the owner of a position. The position must not have an owner.
fn grant_position(&mut self, owner: Address, id: U256) {
// set owner
self.position_owners.setter(id).set(owner);
// increment count
let owned_positions_count = self.owned_positions.get(owner) + U256::one();
self.owned_positions
.setter(owner)
.set(owned_positions_count);
}This could allow unauthorized transfers of position ownership, going against the intention.
Consider adding a check at the beginning of the grant_position function to verify that the position's current owner is Address::ZERO before proceeding with the ownership transfer. Also add a new error msg to handle cases where a position is already owned.
A malicious user could intentionally exhaust storage by creating unlimited positions without requiring any initial liquidity. This issue exists in the position minting functionality where users are allowed to create new positions by only specifying tick ranges without being required to provide any liquidity.
In lib.rs, the position minting function only validates basic parameters:
pub fn mint_position_B_C5_B086_D(
&mut self,
pool: Address,
lower: i32,
upper: i32,
) -> Result<U256, Revert> {
let id = self.next_position_id.get();
self.pools.setter(pool).create_position(id, lower, upper)?;
self.next_position_id.set(id + U256::one());
self.grant_position(owner, id);
// ...
}The pool's create_position function in pool.rs only validates tick spacing:
pub fn create_position(&mut self, id: U256, low: i32, up: i32) -> Result<(), Revert> {
assert_or!(self.enabled.get(), Error::PoolDisabled);
let spacing = self.tick_spacing.get().sys();
assert_or!(low % spacing as i32 == 0, Error::InvalidTickSpacing);
assert_or!(up % spacing as i32 == 0, Error::InvalidTickSpacing);
// ... tick range checks ...
self.positions.new(id, low, up);
Ok(())
}The position creation in position.rs simply stores empty position data:
pub fn new(&mut self, id: U256, low: i32, up: i32) {
let mut info = self.positions.setter(id);
info.lower.set(I32::lib(&low));
info.upper.set(I32::lib(&up));
}Attackers can create unlimited positions with zero liquidity. Each position consumes permanent storage space in the StorageMap<U256, StoragePositionInfo>.
A malicious user could:
- Call
mint_position_B_C5_B086_Drepeatedly with valid tick ranges. - Each call creates a new storage entry in
positionsmapping. - No liquidity is required, making the attack virtually costless beyond basic transaction fees.
- The
next_position_idcounter keeps incrementing, allowing unlimited position creation.
Consider implementing atomic position creation:
pub fn mint_position_with_liquidity(
&mut self,
pool: Address,
lower: i32,
upper: i32,
initial_liquidity: u128,
amount_0_min: U256,
amount_1_min: U256,
) -> Result<U256, Revert> {
let id = self.next_position_id.get();
// Create position
self.pools.setter(pool).create_position(id, lower, upper)?;
// Require immediate liquidity provision
self.adjust_position_internal(
pool,
id,
amount_0_min,
amount_1_min,
initial_liquidity,
None
)?;
// Only grant position if liquidity is added successfully
self.grant_position(msg::sender(), id);
self.next_position_id.set(id + U256::one());
Ok(id)
}Also, consider implementing a minimum liquidity threshold that must be provided during position creation, because we need to prevent zero-liquidity position spam while allowing legitimate small positions.
The contract currently imports the U256 type from two different sources:
use crate::types::U256; // Line 5
use stylus_sdk::alloy_primitives::{Address, U256}; // Line 12U256 type is imported twice from different sources, but only one of these imports is actually being used. This dual import creates potential ambiguity because it's unclear which implementation should be used. The code predominantly uses the stylus_sdk version, particularly in conjunction with the ruint::uint macro in tests and with other stylus_sdk types like Address. Having two imports of the same name can lead to compiler confusion.
Remove the crate::types::U256 import and retain only the stylus_sdk version:
// Remove this line
// use crate::types::U256;
// Keep this line
use stylus_sdk::alloy_primitives::{Address, U256};mul_mod function uses debug_assert!(!overflow) to check for multiplication overflow. Since debug_assert! is only active in debug builds, this critical overflow check is completely removed in release builds. This could lead to silent failures and incorrect calculations in production environments where release builds are typically used.
pub fn mul_mod(a: U256, b: U256, mut modulus: U256) -> U256 {
if modulus == U256::ZERO {
return U256::ZERO;
}
// alloc a 512 bit result
let mut product = [0; 8];
let overflow = ruint::algorithms::addmul(&mut product, a.as_limbs(), b.as_limbs());
debug_assert!(!overflow);
// compute modulus
// SAFETY - ruint code
unsafe { ruint::algorithms::div(&mut product, modulus.as_limbs_mut()) };
modulus
}Consider replacingdebug_assert!(!overflow) with assert!(!overflow) to ensure overflow checks are performed in both debug and release builds; to enable us maintain arithmetic safety across all build configurations.
pub fn mul_mod(a: U256, b: U256, mut modulus: U256) -> U256 {
if modulus == U256::ZERO {
return U256::ZERO;
}
// alloc a 512 bit result
let mut product = [0; 8];
let overflow = ruint::algorithms::addmul(&mut product, a.as_limbs(), b.as_limbs());
- debug_assert!(!overflow);
+ assert!(!overflow, "multiplication overflow");
// compute modulus
// SAFETY - ruint code
unsafe { ruint::algorithms::div(&mut product, modulus.as_limbs_mut()) };
modulus
}af-afk (Superposition) commented:
- [01] - We're happy with this behaviour currently. We think it makes sense in the context with how the initial price and costing could be abused.
- [02] - Could we have some extra context if this is an issue in its application? It's true that the function behaves like this, but it's privately used in the codebase, and it's our understanding that these callers enforce correct checks.
- [03] - We recognise that this is potentially an issue, but we don't perceive it is likely to happen in practice. Even with a small amount, someone could create a position, supply some liquidity, remove it, and do all this in the same function, with the only cost a greater gas profile. A better architectural decision would be to move the position ID behaviour into a per pool basis, but we don't believe that in practice someone will grief this function to that extent.
- [04] - We should change this. Thankfully for us this is the same implementation.
- [05] - We would appreciate some evidence under which circumstances this could cause an issue.
While QA reports are not eligible for rewards on this audit, I believe this QA report is acceptable and thus merits an A rating.
C4 is an open organization governed by participants in the community.
C4 audits incentivize the discovery of exploits, vulnerabilities, and bugs in smart contracts. Security researchers are rewarded at an increasing rate for finding higher-risk issues. Audit submissions are judged by a knowledgeable security researcher and solidity and rust developer and disclosed to sponsoring developers. C4 does not conduct formal verification regarding the provided code but instead provides final verification.
C4 does not provide any guarantee or warranty regarding the security of this project. All smart contract software should be used at the sole risk and responsibility of users.