Skip to content

fix: Fix typo in test file and expose poolKeys in IPositionManager in…#509

Open
Trustdev-eth wants to merge 2 commits into
Uniswap:mainfrom
Trustdev-eth:fix/typo-and-poolkeys-interface
Open

fix: Fix typo in test file and expose poolKeys in IPositionManager in…#509
Trustdev-eth wants to merge 2 commits into
Uniswap:mainfrom
Trustdev-eth:fix/typo-and-poolkeys-interface

Conversation

@Trustdev-eth

Copy link
Copy Markdown

Related Issue

Fixes #[issue-number-1] - Typo in the name of test file
Fixes #[issue-number-2] - IPositionManager doesn't expose poolKeys(bytes32)

Description of changes

This PR fixes two bugs:

  1. Fixed typo in test file name: Renamed DeployPoolMofifyLiquidityTest.t.sol to DeployPoolModifyLiquidityTest.t.sol (corrected "Mofify" → "Modify")

  2. Exposed poolKeys function in IPositionManager interface: Added the missing poolKeys(bytes25 poolId) function declaration to the IPositionManager interface. The PositionManager contract already has a public mapping poolKeys, but it wasn't exposed in the interface, making it inaccessible through the interface.

Changes Made

  • Renamed test/script/DeployPoolMofifyLiquidityTest.t.soltest/script/DeployPoolModifyLiquidityTest.t.sol
  • Added function poolKeys(bytes25 poolId) external view returns (PoolKey memory); to src/interfaces/IPositionManager.sol

Testing

  • Code compiles without errors
  • No linter errors
  • Tests pass (run forge test)

Checklist

  • Code follows the project's style guidelines
  • Changes are documented with natspec comments
  • PR references the related issues

@BhariGowda

Copy link
Copy Markdown

The bytes25 poolId used here is NOT compatible with the PoolId type
from v4-core (which is bytes32). Consider adding a NatSpec warning
to prevent integrator confusion:

/// @dev This poolId is NOT compatible with the PoolId type from v4-core.
/// It is the truncated bytes25 value stored in PositionInfo.

@Trustdev-eth

Copy link
Copy Markdown
Author

The bytes25 poolId used here is NOT compatible with the PoolId type from v4-core (which is bytes32). Consider adding a NatSpec warning to prevent integrator confusion:

/// @dev This poolId is NOT compatible with the PoolId type from v4-core. /// It is the truncated bytes25 value stored in PositionInfo.

Good call — added @dev NatSpec on poolKeys in IPositionManager that this is truncated bytes25 from PositionInfo, not v4-core PoolId (bytes32), aligned with PositionInfoLibrary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants