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

Fix/staking bug #1162

Merged
merged 6 commits into from
Sep 9, 2023
Merged

Fix/staking bug #1162

merged 6 commits into from
Sep 9, 2023

Conversation

unknownunknown1
Copy link
Contributor

@unknownunknown1 unknownunknown1 commented Aug 16, 2023

List of changes made in this PR:

  • Previously drawing iterations were capped with required number of jurors and the argument _iterations didn't really do anything. It makes sense in the old version where we don't have post-draw check and each iteration will draw someone anyway. But in this version we skip jurors, so I decided to remove this cap and use an iteration number (which is ever-increasing) as a nonce for obtaining a drawing number, thus each time you call draw() function it'll draw different jurors in the process, unlike before, since each call will use different nonces.

  • Staking logic now considers the implicit staking in parent courts, thus insolvency is now checked not by the stake in a particular court but by the total number of tokens deposited by the juror. If this number is lower than the total number of locked tokens the juror won't pass the post-draw check.
    Also penalty now doesn't update the stake, instead it simply removes tokens from total juror's balance, which is actually closer to V1 logic-wise.
    So in short, we now have 3 closely related, but different entities:
    1. totalStaked, which reflects the number of tokens deposited by the juror. If the juror is fully unstaked, or for some reason has totalLocked > totalStaked, then locked tokens will reflect it instead
    2. stakedPnkByCourt - number of tokens that were explicitely staked in a particular court. We need it as a duplication of sortition stakes, in case the sortition module is replaced
    3. Sortition Stakes - includes all, explicit stakes and implicit stakes in parent courts.

  • Locked tokens are now taken into account during staking. E.g. if a juror has 0 tokens staked and 150 locked then to stake 1000 he'll only need to transfer 850 tokens. It only applies when total locked amount is higher than total staked

  • Staking 0 isn't allowed if currentStake == 0 to avoid some unexpected behaviour

@netlify
Copy link

netlify bot commented Aug 16, 2023

Deploy Preview for kleros-v2 ready!

Name Link
🔨 Latest commit 7d90db7
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2/deploys/64fc167581419e0008a8cc66
😎 Deploy Preview https://deploy-preview-1162--kleros-v2.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 16, 2023

Deploy Preview for kleros-v2-contracts ready!

Name Link
🔨 Latest commit 6460b34
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-contracts/deploys/64ed917c81a52100088f784c
😎 Deploy Preview https://deploy-preview-1162--kleros-v2-contracts.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jaybuidl jaybuidl marked this pull request as ready for review September 9, 2023 06:54
@codeclimate
Copy link

codeclimate bot commented Sep 9, 2023

Code Climate has analyzed commit 7d90db7 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Sep 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jaybuidl jaybuidl merged commit 7d90db7 into kleros:dev Sep 9, 2023
11 of 12 checks passed
@jaybuidl jaybuidl added this to the testnet-2 milestone Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compatibility: ABI change 🗯 Smart contract ABI is changing. Package: Contracts Court smart contracts Type: Bug 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants