-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add token name to structure #1336
Conversation
@@ -42,7 +44,7 @@ contract BalanceGetter is Simulation { | |||
TokenInfo[] memory results = new TokenInfo[](len); | |||
for (uint256 i = 0; i < len; i++) { | |||
if (tokenAddrs[i] == address(0)) { | |||
results[i] = TokenInfo('ETH', address(account).balance, 18, bytes('')); | |||
results[i] = TokenInfo('ETH', 'Ether', address(account).balance, 18, bytes('')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not directly related to the PR, but how do we handle this for non-eth networks? as far as I remember we change the hardcoded value somewhere in the portfolio library in common
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, it should have been done here it is already done here https://github.com/AmbireTech/ambire-common/blob/v2/src/libs/portfolio/getOnchainBalances.ts#L214
uint256 tokenId = INonfungiblePositionManager(positionManagementAddr).tokenOfOwnerByIndex(userAddr, i); | ||
Position memory positionInfo = INonfungiblePositionManager(positionManagementAddr).positions(tokenId); | ||
|
||
if(positionInfo.liquidity == 0) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice optimization 👍
Resolves: https://github.com/AmbireTech/ambire-app/issues/3951
adds token name property to: