feat: pack ProperQF PackedProject into 1 slot (uint128 + uint128)#400
feat: pack ProperQF PackedProject into 1 slot (uint128 + uint128)#400
Conversation
Code Coverage Report for src/ files
|
8956279 to
17f7bf2
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b69448ef1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// The unchecked downcasts are safe because encode() guarantees the result fits. | ||
| function _writeProject(uint256 projectId, uint256 sumContributions, uint256 sumSquareRoots) internal { | ||
| _getProperQFStorage().projects[projectId] = PackedProject({ | ||
| sumContributions: uint96(CONTRIBUTIONS_SCHEME.encode(sumContributions)), |
There was a problem hiding this comment.
Preserve per-project remainder when quantizing contributions
sumContributions is quantized on every write, and _processVoteUnchecked rebuilds newSumContributions from the already-decoded (floored) value, so sub-step remainders are dropped permanently each vote instead of accumulating. For example, two votes with contribution = STEP/2 still leave sumContributions == 0; this undercounts the linear term in getTally, and QuadraticVotingMechanism uses that linear term for both quorum and share minting, so proposals can be under-allocated or fail quorum whenever vote costs are not step-aligned (especially with smaller weights or lower-alpha settings).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both this and the sibling finding (silent zeroing for small weights) are addressed by the same fix: a ContributionBelowMinimum guard in _processVoteUnchecked that rejects contribution < CONTRIBUTIONS_SCHEME.stepSize() (= 2^32).
Why remainders don't compound: _readProject always returns a step-aligned value (stored << 32), so the floor loss per vote is strictly contribution mod STEP, independent of accumulated history. Remainders from previous votes have already been truncated and don't interact with new contributions.
Bounded loss after the guard: With contribution >= STEP enforced, the per-vote loss ratio is remainder / contribution. For the minimum weight (65536), contribution = 2^32 = STEP exactly, so loss = 0. For realistic weights the loss ratio is negligible (e.g. weight=1e9: ~5e-8% per vote).
Global sums stay exact: totalLinearSum uses the delta pattern old_decoded - old_decoded + (old_decoded + contribution), which cancels the quantization error. Only per-project readback in getTally is lossy, bounded by N * (STEP - 1) across N votes.
Preserving the remainder would require either an extra storage slot per project (defeating the packing goal) or a fundamentally different accumulation strategy. Given the guard makes the worst case precision dust, I think the tradeoff is right.
cda6624 to
a98bbe8
Compare
Compress per-project storage from 2 slots to 1 using symmetric uint128 field widths with lossy quantization for sumContributions via uint-quantization-lib v7.0.1. Layout: uint128 sumContributions (shift=32, lossy) + uint128 sumSquareRoots (lossless) sumContributions: quantized via CONTRIBUTIONS_SCHEME (step=2^32, max~2^160). sumSquareRoots: uint128 ceiling matches its arithmetic constraint (squared on-chain, which overflows uint256 at 2^128). Enforce weight alignment to MIN_VOTE_WEIGHT=2^16 so contribution=weight^2 is always step-aligned, making per-project sums lossless. Enforce 18-decimal input assumption via ProperQF constructor parameter to prevent miscalibration by future inheritors.
a98bbe8 to
dd50d12
Compare
…ation-lib v7.1.0 Replace custom ContributionBelowMinimum error and manual check in ProperQF with requireMinStep() and BelowMinStep error from the library (issue #93).
Reduce per-project ProperQF storage from 2 slots to 1 using uint-quantization-lib v7.0.1.
PackedProject { uint128 sumContributions, uint128 sumSquareRoots }= 256 bits = 1 slot.sumContributions is lossy-quantized (shift=32, step=2^32 wei, effective range 2^160). Contributions operate in the 18-decimal-normalized voting-power space (see
QuadraticVotingMechanism._normalizeToDecimals), so the step size corresponds to ~4.3 nanotoken regardless of the underlying asset's native decimals. sumSquareRoots is stored losslessly; its uint128 width matches the arithmetic ceiling imposed by on-chain squaring (sumSR * sumSRoverflows uint256 at 2^128). Global sums stay exact via delta update cancellation.Weight alignment (
weight % MIN_VOTE_WEIGHT == 0) is enforced on-chain socontribution = weight^2is always step-aligned, making per-project sums lossless. The 18-decimal input assumption is enforced viaProperQF(uint8 inputDecimals)constructor parameter, preventing miscalibration by future inheritors.