Pashov L-04 display Native instead of the zero address #373
Conversation
src/libraries/SafeERC20Metadata.sol
Outdated
| library SafeCurrencyMetadata { | ||
| using CurrencyLibrary for Currency; | ||
|
|
||
| library SafeERC20Metadata { |
There was a problem hiding this comment.
why did you rename this back? it was currency because it also handles native right? why is it ERC20 now?
There was a problem hiding this comment.
mm i renamed it back since we're not unwrapping the currency in the file anymore, its just working with address
There was a problem hiding this comment.
mmm but its not only working with ERC20s either...? its still working with native e.g.
if (addr == address(0)) {
return nativeLabel;
}
i'd personally still rather the name "currency"? its still working with both native and ERC20s even if its not using the Currency type
There was a problem hiding this comment.
AddressRatioSortOrder is fine.. could you address if you prefer. But ERC20 is still wrong imo
src/libraries/SafeERC20Metadata.sol
Outdated
| library SafeCurrencyMetadata { | ||
| using CurrencyLibrary for Currency; | ||
|
|
||
| library SafeERC20Metadata { |
There was a problem hiding this comment.
mmm but its not only working with ERC20s either...? its still working with native e.g.
if (addr == address(0)) {
return nativeLabel;
}
i'd personally still rather the name "currency"? its still working with both native and ERC20s even if its not using the Currency type
src/libraries/SafeERC20Metadata.sol
Outdated
| library SafeCurrencyMetadata { | ||
| using CurrencyLibrary for Currency; | ||
|
|
||
| library SafeERC20Metadata { |
There was a problem hiding this comment.
AddressRatioSortOrder is fine.. could you address if you prefer. But ERC20 is still wrong imo
test/PositionDescriptor.t.sol
Outdated
| assertEq(token.name, "Uniswap - 0.3% - TEST/ETH - 1.0060<>1.0121"); | ||
| assertEq( | ||
| token.description, | ||
| unicode"This NFT represents a liquidity position in a Uniswap v4 TEST-ETH pool. The owner of this NFT can modify or redeem the position.\n\nPool Manager Address: 0x5615deb798bb3e4dfa0139dfa1b3d433cc23b72f\nTEST Address: 0x5991a2df15a8f6a256d3ec51e99254cd3fb576a9\nETH Address: Native\nHook Address: 0x0000000000000000000000000000000000000000\nFee Tier: 0.3%\nToken ID: 1\n\n⚠️ DISCLAIMER: Due diligence is imperative when assessing this NFT. Make sure addresses match the expected addresses, as symbols may be imitated." |
There was a problem hiding this comment.
This should say "No Hook" right? Maybe you didnt merge main
There was a problem hiding this comment.
yeah i didnt merge main!
Related Issue
Pashov L-04 Using zero address for native token can be confusing for end users
Description of changes
display Native instead of the zero address