hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

mismatch in `minWithdrawal` due to no decimal consideration #68

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe85fd8821d4104ce1fe919873ad407699e3f7238a494f9f7864670084fadd094 Severity: low

Description: Description\ as we can see the minimum withdraw request in UI is 1 stROSE token.

image\ but in the code:

uint256 public minWithdrawal = 1; // min withdrawal amount (wei)

its 1 wei of stROSE not 1 stROSE.

according to hats-finance rules:

Issues where the behavior of the contracts differs from the intended behavior (as described in the docs and by common sense), but no funds are at risk.

this is valid Low issue.

Recommendation\ consider decimals for minWithdrawal

uint256 public minWithdrawal = 1e18; // min withdrawal amount (wei)
ilzheev commented 1 month ago

minWithdrawal is updatable, and it was updated in the minter to 1e18. no error

0xdanial commented 1 month ago

@ilzheev thanks for your comment.

Yes, it is updatable, which is why I labeled this issue as low severity. There are no tests for the codebase, and there is no documentation or README mentioning that this will be updated to 1e18. Based on your argument, it would imply that no bugs exist in upgradable contracts because the owner can always upgrade and fix issues. With the codebase and data available, I found a mismatch between the UI and the code, and this should be considered valid Low issue according to Hats-Finance rules:

Issues where the behavior of the contracts differs from the intended behavior (as described in the docs and by common sense), but no funds are at risk.

btw lets see @0xRizwan opinion

ilzheev commented 1 month ago

@0xdanial contract is not upgradeable, but there is method updateMinWithdrawal that can be called – and it was called – so there is no mismatch. minWithdrawal of the deployed contract returns 1e18.

0xdanial commented 1 month ago

@ilzheev upgradability thing i said was for example. there is no available data for me to knew that it was updated to the intended value. none of the deployed contracts on the testnet are verified, there are no tests for the contracts, and there is no information in the documentation or README about this.

It is unfair to mark this issue as invalid based on data that is not available to auditors.