-
Notifications
You must be signed in to change notification settings - Fork 51
feat: add available stake, effectivestake #2098
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
base: dev
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReplaces Courts with Stakes in the Profile page. Introduces a new Stakes Header showing available, staked (effective), and locked PNK. Removes the old Courts Header. Updates Stakes page to compute and pass stake values. Adjusts NumberDisplay tooltip to use commified values for both currency and non-currency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProfilePage as Profile Page
participant Stakes as Stakes Page
participant Header as Stakes Header
participant NumberDisplay
User->>ProfilePage: Navigate to Profile
ProfilePage->>Stakes: Render with { addressToQuery }
Stakes->>Stakes: Compute available, locked, effective from data
Stakes->>Header: Render with { availableStake?, lockedStake?, effectiveStake? }
Header->>NumberDisplay: Render PNK values (commified tooltip)
Note over NumberDisplay: Tooltip uses commify for value display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 3
🧹 Nitpick comments (4)
web/src/components/NumberDisplay.tsx (1)
16-26
: Guard against decimals=0 producing invalid "< 0.1" boundaries
"0".repeat(decimals - 1)
throws fordecimals = 0
. Add a safe clamp and only emit the threshold strings whendecimals > 0
.Apply this diff:
-const getFormattedValue = (value: number, decimals: number) => { - const withFixedDecimals = value % 1 !== 0 ? value.toFixed(decimals) : value.toFixed(0); - if (value !== 0) { - if (withFixedDecimals === `0.${"0".repeat(decimals)}`) { - return `< 0.${"0".repeat(decimals - 1)}1`; - } else if (withFixedDecimals === `-0.${"0".repeat(decimals)}`) { - return `> -0.${"0".repeat(decimals - 1)}1`; - } - } - return withFixedDecimals; -}; +const getFormattedValue = (value: number, decimals: number) => { + const safeDecimals = Math.max(0, decimals); + const withFixedDecimals = value % 1 !== 0 ? value.toFixed(safeDecimals) : value.toFixed(0); + if (value !== 0 && safeDecimals > 0) { + if (withFixedDecimals === `0.${"0".repeat(safeDecimals)}`) { + return `< 0.${"0".repeat(safeDecimals - 1)}1`; + } else if (withFixedDecimals === `-0.${"0".repeat(safeDecimals)}`) { + return `> -0.${"0".repeat(safeDecimals - 1)}1`; + } + } + return withFixedDecimals; +};web/src/pages/Profile/Stakes/index.tsx (1)
65-71
: Minor: avoid double filteringYou already computed
stakedCourts
. Reuse it in the map to avoid repeating the filter.Apply this diff:
- {isStaked && !isLoading ? ( - <CourtCardsContainer> - {stakeData?.jurorTokensPerCourts - ?.filter(({ staked }) => staked > 0) - .map(({ court: { id, name }, staked }) => ( - <CourtCard key={id} name={name ?? ""} stake={staked} {...{ id }} /> - ))} - </CourtCardsContainer> - ) : null} + {isStaked && !isLoading ? ( + <CourtCardsContainer> + {stakedCourts!.map(({ court: { id, name }, staked }) => ( + <CourtCard key={id} name={name ?? ""} stake={staked} {...{ id }} /> + ))} + </CourtCardsContainer> + ) : null}web/src/pages/Profile/Stakes/Header.tsx (2)
97-99
: Avoid boolean-or-string intermediates from&&
chaining
const formattedX = !isUndefined(x) && formatUnits(x, 18)
yieldsstring | false
. It works at runtime but is noisy for types and easy to regress. Preferundefined | string
.Apply this diff:
- const formattedAvailableStake = !isUndefined(availableStake) && formatUnits(availableStake, 18); - const formattedLockedStake = !isUndefined(lockedStake) && formatUnits(lockedStake, 18); - const formattedEffectiveStake = !isUndefined(effectiveStake) && formatUnits(effectiveStake, 18); + const formattedAvailableStake = isUndefined(availableStake) ? undefined : formatUnits(availableStake, 18); + const formattedLockedStake = isUndefined(lockedStake) ? undefined : formatUnits(lockedStake, 18); + const formattedEffectiveStake = isUndefined(effectiveStake) ? undefined : formatUnits(effectiveStake, 18);
119-133
: Label clarity: consider “Staked (effective)”If the intent is to show effective stake (vs raw staked) it may help to disambiguate the label for users.
Apply this diff:
- <label> Staked: </label> + <label> Staked (effective): </label>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
web/src/components/NumberDisplay.tsx
(1 hunks)web/src/pages/Profile/Courts/Header.tsx
(0 hunks)web/src/pages/Profile/Stakes/Header.tsx
(1 hunks)web/src/pages/Profile/Stakes/index.tsx
(3 hunks)web/src/pages/Profile/index.tsx
(2 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/Profile/Courts/Header.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jaybuidl
PR: kleros/kleros-v2#1746
File: contracts/config/courts.v2.mainnet-neo.json:167-170
Timestamp: 2024-11-19T16:31:08.965Z
Learning: In `contracts/config/courts.v2.mainnet-neo.json`, the `minStake` parameter is denominated in PNK, not ETH.
📚 Learning: 2024-06-27T10:11:54.861Z
Learnt from: nikhilverma360
PR: kleros/kleros-v2#1632
File: web/src/components/DisputeView/DisputeInfo/DisputeInfoList.tsx:37-42
Timestamp: 2024-06-27T10:11:54.861Z
Learning: `useMemo` is used in `DisputeInfoList` to optimize the rendering of `FieldItems` based on changes in `fieldItems`, ensuring that the mapping and truncation operation are only performed when necessary.
Applied to files:
web/src/pages/Profile/index.tsx
🧬 Code Graph Analysis (2)
web/src/components/NumberDisplay.tsx (1)
web/src/utils/commify.ts (1)
commify
(1-48)
web/src/pages/Profile/Stakes/Header.tsx (1)
web-devtools/src/styles/responsiveSize.ts (1)
responsiveSize
(9-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
web/src/pages/Profile/index.tsx (2)
15-17
: Style imports reorg looks goodThe consolidated landscape/responsive imports keep styling concerns localized and consistent.
24-24
: Switch to Stakes component aligns with PR intentReplacing Courts with Stakes is consistent with the feature objective and downstream components.
web/src/pages/Profile/Stakes/index.tsx (5)
38-40
: IStakes typing is precise and helpfulExplicitly using the
0x
-prefixed template literal type is great for safety across contract hooks.
42-46
: Confirm courtId=1 for juror balance is correct across chains
useReadSortitionModuleGetJurorBalance({ args: [addressToQuery, BigInt(1)] })
hard-codescourtId = 1
. If balances are court-specific or network-dependent, this could misstate Available/Locked PNK.Would you confirm whether
1n
is always the intended court (e.g., General court) for global juror balance? If not, we should derive the correct courtId from config or aggregate across relevant courts.
51-52
: LGTM: Available/Locked derive cleanly from contract tupleIndexing the contract return for
[available, locked]
keeps things straightforward and type-safe.
59-60
: Header integration looks correctPassing
{ lockedStake, availableStake, effectiveStake }
keeps the presentation layer focused and free of contract concerns.
77-77
: Rename/export are coherentDefault-exporting
Stakes
matches the new import sites.web/src/pages/Profile/Stakes/Header.tsx (2)
96-103
: Title and prop wiring LGTMHeader props line up with the new data flow; the title switch based on the query param is straightforward.
107-115
: Good conditional rendering and consistent NumberDisplay usageGuarding each row with
isUndefined
keeps the UI stable while data loads. UsingNumberDisplay
with unit "PNK" leverages the improved tooltip formatting.
PR-Codex overview
This PR focuses on refactoring the
Courts
component into a newStakes
component, updating the associated UI and data handling for a clearer representation of juror stakes.Detailed summary
Header.tsx
fromCourts
.Courts
toStakes
in several files.tooltipValue
formatting inNumberDisplay.tsx
.Header
component inStakes
with props foravailableStake
,lockedStake
, andeffectiveStake
.Profile
component to renderStakes
instead ofCourts
.Summary by CodeRabbit
New Features
Refactor