Account for natural precision loss in matching pool distribution#339
Account for natural precision loss in matching pool distribution#339akaia-shadowfox merged 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request refines the structure of various voting and payout hooks and components. It modifies the return structure of elections hooks to utilize a unified Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component
participant VRHook as VotingRoundResultsHook
participant Config as potContractHooks.useConfig
participant Validator as revalidate()
UI->>VRHook: Request voting round results
VRHook->>Config: Fetch pot configuration
Config-->>VRHook: Return potConfig
VRHook->>Validator: Process winners & calculate payouts with token metadata
Validator-->>VRHook: Return computed results
VRHook-->>UI: Provide updated voting round data
sequenceDiagram
participant UI as Funding Component
participant Manager as PayoutManager
participant Submit as submitPayouts()
UI->>Manager: Initiate payout submission
Manager->>Submit: Call function with potId & recipients
Submit-->>Manager: Return confirmation
Manager-->>UI: Display payout result
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/entities/voting-round/model/results.ts (3)
62-63: Direct assignment ofpayoutTokenMetadata.
The assignment ensures consistent usage of the updated token metadata. Consider adding a null-check or fallback for edge cases.
154-195: Remainder-based allocation logic.
Granting leftover funds to the last candidate is a valid approach if that is the intended design. However, verify it aligns with stakeholder expectations—smaller rounding or distribution logic might require a different approach.
200-205: State update with new winners and payout token.
The store update is consistent with the newly computedwinnersandpayoutTokenMetadata. Consider adding dedicated unit tests to ensure end-to-end validation of final state contents.src/entities/pot/hooks/feature-flags.ts (1)
7-10: Renamed de-structured property.
Usingdata: electionskeeps the code semantically clear, but confirm it's consistent throughout the project to avoid confusion in naming conventions.src/common/lib/format.ts (1)
12-13: Consider improving type safety.While the function correctly handles precision using
Big.js, consider adding runtime validation to ensure the result is a validIndivisibleUnits(non-negative integer string).export const bigNumToIndivisible = (amount: Big.Big, decimals: number): IndivisibleUnits => - amount.mul(Big(10).pow(decimals)).toFixed().toString(); + amount.mul(Big(10).pow(decimals)).toFixed(0).toString();src/entities/voting-round/components/leaderboard.tsx (1)
87-91: LGTM! Improved token amount precision handling.The change properly accounts for token decimals when displaying amounts, ensuring accurate representation of values.
Consider extracting the decimal places (2) to a constant to maintain consistency across the application:
+const DISPLAY_DECIMAL_PLACES = 2; + caption={indivisibleUnitsToFloat( estimatedPayoutAmount, NATIVE_TOKEN_DECIMALS, - 2, + DISPLAY_DECIMAL_PLACES, )}src/features/proportional-funding/components/payout-manager.tsx (1)
33-34: Track the TODO comment about Pinata upload.The TODO comment about uploading via Pinata should be tracked to ensure it's not forgotten.
Would you like me to create an issue to track this TODO item?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/common/contracts/core/voting/hooks.ts(2 hunks)src/common/lib/format.ts(1 hunks)src/entities/pot/hooks/feature-flags.ts(1 hunks)src/entities/voting-round/components/WinnerRow.tsx(2 hunks)src/entities/voting-round/components/leaderboard.tsx(2 hunks)src/entities/voting-round/hooks/results.ts(4 hunks)src/entities/voting-round/hooks/rounds.ts(1 hunks)src/entities/voting-round/index.ts(1 hunks)src/entities/voting-round/model/results.ts(4 hunks)src/entities/voting-round/types.ts(2 hunks)src/features/pot-configuration/components/editor.tsx(1 hunks)src/features/proportional-funding/components/payout-manager.tsx(2 hunks)src/features/proportional-funding/model/effects.ts(1 hunks)src/pages/pot/[potId]/history.tsx(1 hunks)src/pages/pot/[potId]/votes.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/pot-configuration/components/editor.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Fleek - broad-haze-5260
- GitHub Check: Fleek - broad-haze-5260
🔇 Additional comments (21)
src/entities/voting-round/model/results.ts (5)
7-10: Imports look valid and consistent.
They're properly typed and grouped, aligning with the new metadata handling logic.
16-16: Schema version bump is correct.
Increasing the schema version to 3 ensures stored data is differentiated and avoids conflicts with the old structure.
23-29: Cache shape updated with new payout token metadata.
The addition ofpayoutTokenMetadatais consistent with the new calculations, but ensure all upstream references are updated accordingly.
38-38: Confirm presence of decimals in NativeTokenMetadata.
Because the union type can includeNativeTokenMetadata, verify that it has adecimalsproperty. Otherwise, runtime errors may occur duringbigNumToIndivisible.Would you like me to generate a shell script to check for fields in
NativeTokenMetadatadefinitions across the codebase?
54-54: Parameter passthrough looks correct.
ForwardingmatchingPoolTokenMetadatato local scope aligns with the new distribution logic. No issues noted.src/entities/pot/hooks/feature-flags.ts (2)
15-15: Usage of optional chaining.
(elections?.length ?? 0) > 0correctly ensures safe checks against undefined. Well done.
17-18: Dependency array updated for re-render control.
Includingelections?.lengthensures the memo recalculates when the number of elections changes. This prevents stale data.src/entities/voting-round/index.ts (1)
8-8: File export casing check.
Confirm that the underlying file is indeedleaderboard.tsorleaderboard.tsxto avoid case-sensitivity issues on certain filesystems.src/features/proportional-funding/model/effects.ts (1)
11-11: LGTM! Simplified payout submission by using pre-converted amounts.The changes correctly handle precision by using
estimatedPayoutAmountdirectly, as it's already inIndivisibleUnitsformat, eliminating the need for conversion and potential precision loss.Also applies to: 17-17
src/entities/voting-round/hooks/rounds.ts (1)
13-16: LGTM! Improved hook return structure.The changes correctly restructure the data return format while maintaining proper dependency tracking in the
useMemohooks.Also applies to: 21-24, 30-32, 37-40
src/entities/voting-round/types.ts (1)
63-63: LGTM! Improved type safety for token amounts.The change from
numbertoIndivisibleUnitsforestimatedPayoutAmountcorrectly enforces proper handling of token amounts and prevents precision loss.src/pages/pot/[potId]/history.tsx (1)
20-21: LGTM! Clean destructuring of hook data.The changes improve code clarity by explicitly showing the data structure returned by the hooks.
src/entities/voting-round/components/WinnerRow.tsx (1)
94-98: Verify decimal place consistency between caption and tooltip.The caption shows 2 decimal places while the tooltip shows full precision (NATIVE_TOKEN_DECIMALS). This might be confusing when values don't match exactly.
Consider either:
- Using the same precision in both places
- Adding a comment explaining the intentional difference
- Adding "(approximate)" to the caption to indicate it's rounded
caption={`~ ${indivisibleUnitsToFloat( estimatedPayoutAmount, NATIVE_TOKEN_DECIMALS, - 2, + NATIVE_TOKEN_DECIMALS, // Match tooltip precision )}`}Also applies to: 107-114
src/features/proportional-funding/components/payout-manager.tsx (1)
40-40: LGTM! Simplified payout submission.The removal of tokenDecimals parameter aligns with the broader refactor for consistent token decimal handling across the application.
src/common/contracts/core/voting/hooks.ts (2)
116-127: LGTM! Consistent data property usage.The refactoring improves consistency by using the
dataproperty pattern for query results.
129-143: LGTM! Consistent data property usage.The refactoring aligns with the same pattern used in
usePotElections.src/entities/voting-round/hooks/results.ts (3)
6-8: LGTM! Improved token handling imports.The new imports support better token metadata handling.
Also applies to: 11-11
23-33: Consider additional error handling for token metadata.While the token handling is improved, consider adding error handling for cases where token metadata might be undefined or invalid.
91-97: Verify token decimals usage.Ensure that
NATIVE_TOKEN_DECIMALSis appropriate for the matching pool token.Consider using the token's own decimals from metadata:
- NATIVE_TOKEN_DECIMALS, + matchingPoolToken.metadata.decimals,✅ Verification successful
I'll start by gathering more context around how
NATIVE_TOKEN_DECIMALSis defined and how it's used alongside the matching pool token metadata. This will help verify if the constant is appropriate for the matching pool token or if the token-specific decimals should be used.
Action: Update matching pool balance conversion to use token-specific decimals.
The search confirms that while several components (e.g., in DonationSuccess.tsx) follow a pattern using the token’s metadata decimals (with a fallback to NATIVE_TOKEN_DECIMALS), the voting round results currently always use NATIVE_TOKEN_DECIMALS. Since
matchingPoolToken.metadatais available, it would be more accurate and consistent to use itsdecimalsproperty for the conversion rather than the generic native value.Recommendations:
- Replace the hardcoded
NATIVE_TOKEN_DECIMALSin the call toindivisibleUnitsToBigNumwithmatchingPoolToken.metadata.decimals.- Optionally, add a fallback in case
matchingPoolToken.metadata.decimalsis missing (e.g., using the nullish coalescing operator).- NATIVE_TOKEN_DECIMALS, + matchingPoolToken.metadata.decimals ?? NATIVE_TOKEN_DECIMALS,🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts or web queries found
Script:
#!/bin/bash # Search for how NATIVE_TOKEN_DECIMALS is defined and its occurrences echo "Searching for definitions and usages of NATIVE_TOKEN_DECIMALS..." rg "NATIVE_TOKEN_DECIMALS" -n . # Search for matchingPoolToken metadata usages in the code echo "Searching for usages of matchingPoolToken.metadata..." rg "matchingPoolToken\.metadata" -n .Length of output: 3094
src/pages/pot/[potId]/votes.tsx (2)
152-163: LGTM! Improved loading state feedback.The addition of a loading state with appropriate visual feedback enhances user experience.
170-170: LGTM! Consistent title casing.The title case update improves text consistency.
* Reimplement wallet bindings & Introduce performance optimizations (#332) Co-authored-by: Jiku Godwill Nsanwi <jikugodwill@yahoo.com> Co-authored-by: Ebube111 <ebubeagwaze@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Account for natural precision loss in matching pool distribution (#339) * fix: Unlock donation submit with default value (#340) * Implement payout justification publishing (#342) * Fix Campaign Issues (#341) * campaign changes done * implemented suggested changes --------- Co-authored-by: Carina.Akaia.io <cvo.akaia@gmail.com> Co-authored-by: Jiku Godwill Nsanwi <jikugodwill@yahoo.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style
Refactor