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

Remove staked contracts check #1400

Open
wants to merge 1 commit into
base: release-hotfix
Choose a base branch
from
Open

Conversation

bobo-k2
Copy link
Contributor

@bobo-k2 bobo-k2 commented Oct 18, 2024

Pull Request Summary

Removed number of staked contracts check from canStake method since we are calling cleanupExpiredEntries before staking, if needed.

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tested on mobile devices

Copy link

Visit the preview URL for this PR (updated for commit a0ae524):

https://astar-apps--pr1400-fix-stake-validation-0ihsi0zb.web.app

(expires Fri, 25 Oct 2024 12:49:19 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db

Comment on lines -448 to -456
} else if (
constants.value &&
(ledger.value?.contractStakeCount ?? 0) >= constants.value.maxNumberOfStakedContracts
) {
return [
false,
t('stakingV3.dappStaking.TooManyStakedContracts'),
docsUrl.dappStakingStaked16Dapps,
];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just informative - the check isn't completely wrong, just incomplete.
You should also check that the dapp on which the user is trying to stake doesn't have an existing DB entry for the user.

If it does, the call will work, because existing entry is being modified, which won't increase number of staked contracts.
If a new contract is being staked on, one for which the staker doesn't have an entry in the DB, then the call will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will update the code

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.

4 participants