Skip to content
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

bug: create pool with rogue base token does not fail #366

Closed
gabririgo opened this issue Nov 4, 2023 · 0 comments · Fixed by #368
Closed

bug: create pool with rogue base token does not fail #366

gabririgo opened this issue Nov 4, 2023 · 0 comments · Fixed by #368
Labels
bug Something isn't working

Comments

@gabririgo
Copy link
Contributor

gabririgo commented Nov 4, 2023

Pool creation with a rogue base token should fail, as asserted by this condition.
When the base token address is a smart contract the transaction should fail, however it won't fail if the address has no code, i.e. a standard wallet. In this case, the decimals are not returned correctly here and the following assertion is bypassed as the token decimals do not get overwritten.
One possible solution is to assert that the target base token is a contract, i.e. address has code. This will result in token.decimals() call to revert if the target contract does not implement decimals() method.

IRigoblockPoolProxyFactory.Parameters memory initParams = IRigoblockPoolProxyFactory(msg.sender).parameters();
uint8 tokenDecimals; // we do not initialize decimals

if (initParams.baseToken != address(0)) {
   assert(initParams.baseToken.code.length > 0) // this guarantees that if the token does not implement decimals the call will fail
   tokenDecimals = IERC20(initParams.baseToken).decimals();
   assert(tokenDecimals >= 6); // we move the assertion inside the block as otherwise we know decimals is 18
} else { tokenDecimals = 18; } // we write eth decimals otherwise
@gabririgo gabririgo added the bug Something isn't working label Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant