DAO Project Proposal Feature - #114
Open
jairajdev wants to merge 27 commits into
Open
Conversation
First commit of DAO Phase 2. Types and data only — no handlers, and no proposal can reach a project status yet, so behaviour is unchanged. - Add 'project' to DaoProposalType and 'executing'/'completed'/'terminated' to DaoProposalStatus. The first three are parameter-proposal states; the last three are project-only. - Add DaoMilestone, DaoProjectData, DaoTerminateVote and DaoProjectLogEntry. Endorsements are a flat address array with the proposer at index 0, matching the policy shape; proposedTime/endorsedTime are shared by milestone start and end and must be cleared on each commit. - Hang project data off DaoProposalAccount rather than a new account type, and leave it lazily created like unapplyVotes, so non-project proposals carry no empty object. - Widen the dao_claim_reward and dao_burn_reward status allowlists to include the project statuses. Not cosmetic: a project leaves 'accepted' at dao_project_start and never returns, so without this its voter reward pool would be permanently unclaimable and unburnable. The allowlists are hand-maintained string comparisons that widening the union does not flag. dao_proposal_create still rejects 'project' at runtime; accepting it is the next commit.
Project proposals can now be created, reviewed and voted on. They cannot yet be executed — dao_project_start and the milestone transactions come next. - Route payload validation by type. Projects carry milestones, not parameter changes, so they never reach validateProposalChangeSets, which would reject them for having no change sets at all. - Add validateProjectMilestones with creation-time bounds: milestone count, title and text lengths, a positive finite duration, and parseable non-negative USD strings. The proposer picks the size of an account that every later project transaction rewrites and re-hashes, so this is what stops one proposal making all of its own transactions expensive. - Reject emergency project proposals. Projects mint new coins, so they must always face a community vote rather than the committee-only path. - Require exactly two ballot options for projects. A project has one flat milestone array with no per-option variant, so a third option would select nothing. - Snapshot the bonus and penalty percentages onto the project at creation, so a project is judged by the rules it was created under. validateProjectMilestones deliberately does not validate the contractor address: importing the utils barrel from a fresh util pulls in the existing config/utils import cycle and leaves libToWei undefined at config evaluation time. The address is a sibling field, so the caller checks it.
Standalone commit: this is the only guard on the one DAO operation that creates LIB, so it gets reviewed on its own before dao_project_start consumes it. - Add daoMaxMintThresholdLibStr to LiberdusFlags as a decimal LIB string, so it stays JSON-safe over /debug-liberdus-flags and parses to exact wei with no float rounding. - maxMintThresholdWei() throws on a malformed or negative value instead of falling back to a default. A ceiling that silently becomes something other than what an operator configured is worse than a failed transaction; the throw lands inside transaction validation, so a bad value stops mints rather than widening them. - exceedsMintThreshold() is strictly greater, so a mint exactly at the ceiling is allowed. Zero is a valid value and blocks every mint — a deliberate kill switch, distinct from a malformed one. Two limits are deliberate and both are commented at the flag. This is a per-project cap, not a supply cap: without current_supply, N projects can each pass it and still mint arbitrarily much in aggregate. And the default value is a placeholder that must be set from tokenomics before this reaches a real network.
The mint. An accepted project proposal moves to executing, its balance is minted, and the USD/LIB rate is fixed for every later payout. This is the only DAO transaction that creates LIB, so it is its own commit. - Committee-only, and the grace period always applies. Projects can never be emergency proposals, so unlike dao_apply_parameters there is no path that skips the wait. - Mint is sum(cost + bonus) across milestones, deliberately excluding penalties: a penalty only reduces what a contractor is paid, so folding it in would inflate the escrow and mint more than the project can legitimately pay out. - The amount is recomputed in apply() rather than carried over from validate(), so apply() depends only on wrappedStates, which Shardus snapshots identically for every node. - A malformed milestone amount and a malformed mint ceiling both fail the transaction. Never mint on a total that could not be computed. - rateUsdStr is snapshotted from the network's stability factor. Every later payout converts at this rate, so the contractor carries the LIB price risk from here and the DAO's exposure is fixed at the amount minted. - Start the project log, which every subsequent project transaction appends to. appendProjectLog lives in its own util so the eight project transactions share one implementation. It is uncapped, with the reasoning and a TODO at the function: the log grows with committee behaviour, not milestone count, since re-proposing a time or address is unlimited and each attempt appends.
dao_project_milestone_start, _end and _terminate, plus the endorsement machinery the three of them share. - applyEndorsement implements D7: the proposer always counts as the first endorsement, so three total means three agreeing parties, never four. Read the policy's "endorsed by two committee members", "two more committee members" and "once three committee members submit" with the proposer counted and all three already say three. - The threshold clamps to what is reachable, and the reachable maximum differs by path. On start and end the contractor occupies index 0 and the committee supplies the rest, so the ceiling is committeeSize + 1; clamping those to committeeSize would commit a milestone one endorsement early on a small committee. - The contractor may propose but not endorse. Otherwise they could hold two of the three slots, and re-proposing would let them reset the count every time the committee got close to agreeing. - Re-proposing clears the endorsement list and re-seeds it with the new proposer, and committing clears both fields, so endorsements collected for a start cannot carry into the end. The two questions share the same storage. - Milestones run strictly in order. canStartMilestone checks every earlier milestone rather than only the previous one, which costs nothing and closes the case where an earlier one was left pending. - Milestone numbers are 1-based in transactions and 0-based in storage, with both boundaries rejected explicitly: an off-by-one here misroutes a payment. - Terminating releases the milestone's cost and bonus back out of the project balance, mirroring what was minted for it, and requires a reason on every submission — the log is what a dispute would be argued from. validate() dry-runs the endorsement against a copy of the list so it reports the same rejection apply() would, without mutating consensus state.
The payout. A completed milestone pays the contractor from the project balance, adjusted for how early or late it was delivered. - classifyDelivery compares actual against planned duration using the project's own percentages. Both comparisons are strict, so landing exactly on a threshold is on time — the DAO neither pays a bonus nor levies a penalty for a boundary case. - Early pays cost + bonus, on time pays cost, late pays cost - penalty with no bonus. The result floors at zero: a penalty larger than the cost reduces the payment to nothing but never makes the contractor owe the DAO and never adds back to the balance. Without the floor a large penalty inverts into a credit. - Conversions use usdToWeiAtRate against the project's stored rate, never the live one, so the DAO's exposure stays capped at what it actually minted. - The balance caps the payout regardless of what the milestone arithmetic says. - `paid` records the amount, not a boolean, so a zero payout from a heavy penalty still settles the milestone and `paid > 0n` blocks a second claim. Also fixes a bug in dao_project_milestone_terminate from the previous commit: it released a terminated milestone's escrow at the live rate while the mint had used the project's fixed rate, so the balance drifted whenever the stability factor moved. It now releases at the project rate, mirroring exactly what was minted, and no longer needs the network account at all.
dao_project_change_address, dao_project_end and dao_project_reclaim_balance — the three transactions that manage a project outside its milestones. - change_address is committee-only and, per D14, not allowed while the project is merely accepted. Before dao_project_start there are no funds to redirect and the community voted on a proposal naming that contractor; substituting another party then changes what was approved without a vote. It stays open after the project ends only while a balance remains, since the contractor can still be claiming completed milestones. - end trims the balance to exactly what completed-but-unclaimed milestones are still owed, rather than zeroing it, because the contractor keeps claiming after the project ends. Terminated and already-paid milestones release. - end takes the last milestone's status (D4). Noted at the code: this can under-report failure, since a project whose earlier milestone was terminated but whose last one completed reads as completed. - reclaim_balance zeroes what is left after daoProjectReclaimDelayMs (90 days). Nothing is transferred — the balance was minted into the project and simply ceases to exist, because an unclaimed balance would otherwise inflate the supply forever for work that was never paid for. dao_project_end and dao_project_reclaim_balance carry no fields beyond from and proposalId, so they reuse schemaDaoProjectStartTX rather than duplicating an identical shape three times.
- Add dao/projects/:id, which returns the milestones, balance and rate but omits the log. dao/proposals/:id already returns the whole account, and for a project that is dominated by an unbounded logs array — a caller polling milestone progress should not pull the entire audit trail every time. logCount is returned so a client knows whether fetching it is worth it. - Add dao/projects/:id/logs for the trail itself, fetched deliberately. - Register both after dao/proposals/:id, following the LIFO note already in api/index.ts. - Add client commands for the full lifecycle: dao project <n> and logs, start, end, reclaim, address, and the four milestone verbs. The propose-or-endorse commands take a blank input to endorse what is pending and a value to propose a replacement, mirroring how the transactions themselves distinguish the two. - Add the three project statuses to VALID_DAO_STATUSES so `dao proposals executing` works as a filter. That array is hand-maintained and not derived from DaoProposalStatus, so TypeScript cannot catch an omission.
Five fixes from review, three of them real bugs. - Declare `project` in schemaDaoProposalCreateTX. The schema allowed proposalType 'project' but never declared the payload, and additionalProperties is false — so enabling AJV validation would have rejected every project proposal. Latent today since the flag is off, which is exactly why unit tests did not catch it. - Let the contractor claim after the project ends. loadProjectTxContext required 'executing', but dao_project_end deliberately trims the balance to what completed-but-unclaimed milestones still owe so the contractor can collect it. The code contradicted its own design. loadProjectTxContext now takes the acceptable statuses, defaulting to 'executing' for everything else. - Use `paid > 0n` as the settled marker, in the claim handler and in dao_project_end's unclaimed calculation. This is sound only while no payout can be zero, which the `penalty < cost` creation rule and the start-time wei guard establish — both land in later commits, so a zero payout is still reachable here and would leave such a milestone claimable forever. - Clear terminateVotes when a milestone completes. The plan said so and the implementation did not: abandoned termination intent could otherwise linger on a finished milestone and later reach the threshold, terminating work that was already accepted and paid for. - Let the client create project proposals. The type was in the transaction layer but not in the create prompt, so there was no path to submit one. Milestones and the contractor address are prompted only for projects, and the change-set prompt is skipped for them.
All eight project handlers had the wrong createFailedAppReceiptData contract, so a failed project transaction cost its sender nothing. The dispatcher calls it as (tx, txTimestamp, txId, wrappedStates, dapp, applyResponse, reason) returning void, and the function is responsible for both deducting the fee and attaching the receipt. These handlers declared (tx, txId, txTimestamp, reason) returning the receipt, which meant three things went wrong at once on every failure: - no fee was deducted, so failing was free and repeatable at no cost - txTimestamp and txId were transposed, and wrappedStates landed in the reason parameter - the returned receipt was discarded, so no failure receipt was attached at all Rewritten to match transfer exactly: charge the fee, or the sender's whole balance when that is smaller, stamp the account, and attach the receipt through dapp.applyResponseAddReceiptData. The success paths now go through the same API instead of assigning applyResponse.appReceiptData and appReceiptDataHash directly. That was functionally equivalent — the core method only sets those two fields — but every other handler uses the API, and going direct would silently skip anything core later adds to it.
- reject penalty >= cost in validateProjectMilestones, naming both amounts - drop the zero-cost allowance, which the new rule makes unreachable - keep every late payout strictly positive, so a claim can never settle at zero A penalty is meant to reduce a payment rather than erase it; a milestone where lateness forfeits everything is better expressed as a termination. Since penalty >= 0 was already enforced, penalty < cost implies cost > 0, so the zero-cost case falls out rather than needing its own check.
- add degenerateMilestoneAtRate, repeating the penalty < cost rule in wei - call it in dao_project_start before the rate is snapshotted and the mint made - catch pairs that are valid as USD strings but truncate together at the rate Creation compares decimal strings, but every payout is a truncating division by the project's rate, so amounts that differ in USD can land on the same wei value. The rate is unknown at creation and known at start, and is fixed for the project's life once snapshotted, so one check here covers every later payout. A project can now pass creation and still fail to start. That is the intended outcome: better than minting escrow against milestones that cannot pay out.
- replace the claimed-marker guard with tests for the penalty < cost rule - assert a zero cost is rejected and a late payout stays strictly positive The old guard asserted that `paid` could not double as the settled marker. That premise no longer holds now that a zero payout is unreachable, so the tests cover the two rules that make it unreachable instead.
- add writeOnceError, applied by the milestone paths before they endorse - reject a second proposed time, which also enforces the contractor's one call - read the pending value before writing it, so a first proposal is not mistaken for a re-proposal An endorsement binds to a milestone rather than to a value, so it could be retargeted: a re-proposal landing between a sender forming an endorsement and it being processed silently converted an endorsement of one time into an endorsement of another. Forbidding a second proposal closes that, since what a sender endorses can no longer change under them. It also enforces the policy's "the contractor can only call this once", which applyEndorsement's contractor check did not cover — nothing stopped them re-proposing, and every proposal resets the count, so they could stall their own milestone indefinitely. The check sits in the callers rather than in applyEndorsement because the contractor address path must not have it: proposedAddress is cleared only on a successful commit, so a first proposal nobody endorses would freeze the contractor address for the life of the project, and changing that address is the remedy for a lost or compromised contractor key. Policy line 353 provides for re-proposal there for that reason. The same race therefore stays open on that path, bounded by the threshold: a redirected endorsement still leaves two members who actually chose the committed address.
- drop the status check and allowedStatuses parameter from the loader - state the status rule in each of the eight handlers, beside its other guards - return a discriminated union so callers cannot reach the accounts on error - adopt the loader in start, change address and reclaim, which loaded by hand The eight project transactions use five different status rules between them, so a shared default fitted half of them and had to be overridden by the rest. That default also caused the bug the loader existed to prevent: it silently applied "executing" to the claim handler and blocked the post-end claim that dao_project_end trims the balance for, with nothing at the call site showing which statuses were allowed. Removing it leaves nothing project-status-specific in the loader, so all eight handlers can share it rather than five. Behaviour is unchanged — each inline rule matches the status set its handler had before.
- add findNextPendingMilestone and findExecutingMilestone - drop milestoneNumber from the start and end transactions, schema and client - report the derived number in receipts and logs, so neither loses which milestone was acted on - keep milestoneNumber on terminate and claim, which the policy requires The policy names a different rule per transaction: "start the next milestone" and "end the current milestone" identify a milestone by state, while terminate and claim name one explicitly. All four had been flattened into "the sender supplies a number". Both derivations are pure functions of wrappedStates, so every node resolves the same milestone. A start can still retarget if its milestone finishes between the sender forming the transaction and it being processed, but canStartMilestone then rejects the new target because its predecessor is not finished; C5's write-once rule closes the matching hole on the value being endorsed.
- replace the formatted params string with a typed key-value object - type txType as DaoProjectTxType rather than a bare string - record what identifies an action, not what resulted from it - render the object as key=value pairs in the client The trail exists for disputes between the contractor and the DAO, so a reader should be able to query it rather than parse a sentence. Amounts minted, paid, owed and reclaimed are dropped: they are products of the handler and already recoverable from the account state and the transaction receipt. Keeping them out also means no value in params is ever a bigint. Milestone start and end still record their milestone number even though C3 derives it, since an entry that cannot say which milestone was acted on is not much of an audit entry.
- log the address a sender backed, resolving it from the pending value when the transaction omits it - reject rather than guess when more than one milestone is executing - move the audit-trail doc comment back onto DaoProjectLogEntry A blank endorsement is legal on the address path, so logging tx.proposedAddress directly recorded undefined — off-type for the params record, dropped entirely on serialisation, and it left the trail unable to say which address a member endorsed. That is the one fact a dispute turns on, so it is resolved from the pending value and captured before a commit clears it. findExecutingMilestone returned the first match. Two executing milestones should be unreachable while canStartMilestone holds, but "the current milestone" has to mean one milestone, and silently ending the earliest of several would write a payout against the wrong one.
- milestonePayoutWei takes the project instead of two percentages and a converter - read the rate, bonus and penalty percentages from it directly - note on projectMintAmountWei why the mint still injects its converter All three call sites passed the same four arguments, every one of them derived from the project, and the injected converter bought nothing here: usdToWeiAtRate lives in the same module. What it did buy was the chance to pass a converter bound to the live rate rather than the rate snapshotted at mint — the bug that shipped in dao_project_milestone_terminate and was caught by audit rather than by a test. Taking the project makes it unexpressible. dao_project_milestone_terminate keeps calling usdToWeiAtRate directly. It releases escrow rather than paying out — cost plus bonus at the stored rate, mirroring what the mint put in — so it never goes through milestonePayoutWei, which would wrongly apply delivery speed to a release.
- add planMilestoneTimeEndorsement and planAddressEndorsement - each reads the pending value and endorsement list itself, and returns the next state instead of mutating - validate and apply call the same function, so they cannot disagree - keep applyEndorsement and writeOnceError for their own unit tests validate dry-ran the endorsement against a copy while apply ran it for real, so the same six positional arguments were written twice per handler and had to agree. They diverged once already: apply read "is a value pending" after writing proposedTime, so every opening proposal looked like a re-proposal, the error was discarded, and no endorsement was ever seeded. Nothing below the E2E could catch it. applyEndorsement takes that flag as a parameter, so its tests passed throughout and would pass again. Reading the state inside the helper and applying only what it returns removes the caller's opportunity to mutate first, and the new tests target the function the handlers actually call. Two helpers rather than one: the paths differ in value type, in whether the contractor may take part, and in whether write-once applies. A single helper would take all of that as flags, which is the indirection this removes.
- require both timestamps and endTime >= startTime before computing a payout - say when the milestone order check fires, not just that it must run - give every project status rejection the shape the rest of the repo uses A missing timestamp used to default to zero, which made the duration hugely negative, classified the milestone as early, and paid cost plus bonus. The least trustworthy data earned the most generous outcome. It is unreachable today — dao_project_milestone_end sets endTime and the completed status together — but the failure mode was backwards. endTime >= startTime is a separate check rather than an implication of both being present: the two times are proposed and endorsed independently, and the end time is only bounded above by the transaction timestamp. Equal times are a legitimate zero-length milestone; inverted ones are not. Callers already route this correctly — the claim path turns it into a rejection in validate and throws in apply, and dao_project_end throws, since a corrupt milestone there is a broken invariant rather than a user error. The order check note records what findNextPendingMilestone cannot catch: the next pending milestone may still sit behind one that is executing. That rejection is what makes deriving the milestone safe, so it should not be dropped as redundant.
- cut restatements of what the code plainly does - keep every invariant a reader cannot recover from the code The utils sat at 43-51% comment lines against 3-8% in the handlers, with several docs longer than the function beneath them. What survives is the reasoning that took a bug to learn: payouts convert at the rate snapshotted at mint and never the live one, penaltyUsdStr is an amount while durationPenaltyPercentage is the trigger, paid > 0n is sound only because a zero payout is unreachable, write-once covers milestone times but not addresses, and why one module injects its converter while the other takes the project.
Scenario 21 drives a four-milestone project from creation to reclaim, covering the paths that only exist on a live network. - Creation negatives: emergency projects, three-option project ballots and milestones whose penalty is not smaller than their cost are all rejected. - dao_project_start is refused before the vote and from a non-committee sender, then asserts the mint is sum(cost + bonus) — penalties excluded — converted at the project's own stored rate. Checking against project.rateUsdStr rather than the live factor also proves the rate was snapshotted at all. - Milestone ordering is enforced by the derivation itself: start and end no longer send a number, so the 1-based boundary checks are asserted on the claim path, which still names a milestone. - Write-once: once a start time is proposed, neither another committee member nor the contractor can replace it, and the original survives the attempt. - Endorsement flow end to end: the contractor proposes, cannot endorse their own proposal, two committee endorsements commit it, and the endorsement list is cleared so nothing carries into the end question. The four milestones each exercise a different path, chosen so the two fixes from review are tested rather than assumed: - 1 delivered early, claimed while executing: pays cost + bonus, second claim rejected. - 2 terminated by three distinct committee members — one member voting twice is rejected — releasing exactly cost + bonus at the project rate, mirroring the mint. The contractor then cannot claim it. - 3 delivered late with a penalty below its cost, paying cost - penalty. The test asserts the exact payout, that the balance falls by precisely that much, and that a second claim is rejected — `paid > 0n` settles the milestone, which is sound now that no payout can be zero. - 4 completed but left unclaimed until after dao_project_end, so the widened claim allowlist is exercised. The project ends with a balance equal to what milestone 4 is owed, which is then claimed while the project reads completed. Both halves of D9 are covered, and their placement is forced by the fixture: the claim window is 150s from voting end while the project lifecycle runs for minutes, so the reward claim happens as soon as the project reaches executing — which is also the exact status the allowlist was missing. A second voter never claims, leaving a residue that the tail step burns from a completed project. Money expectations mirror the handler's arithmetic term by term, not just its net USD, because every conversion truncates. Sums go through usdSumToLibWei, one amount at a time, since the handlers convert each milestone's cost and bonus separately. The late payout subtracts two separately converted amounts for the same reason: floor(cost) - floor(penalty) can be a wei below floor(cost - penalty), so converting the difference would not match. Contractor address changes are covered separately, following the policy: a proposal, a blank endorsement of it, a re-proposal that resets the count, a duplicate endorsement rejected, and a non-committee sender rejected. It deliberately stops short of three endorsements, since committing would replace the contractor and break every later claim. Also asserts the under-reporting D4 accepts (the project reads completed despite milestone 2 being terminated) and that reclaim is refused once the balance is zero. sequentialOnly: the milestone chain depends on each endorsement having committed before the next step, and it asserts on balances only this scenario moves.
- drop timestamp: Date.now() from 129 transaction literals - say on refreshTxTimestamp that it owns the field, and what to do instead - correct injectLate's reason, which no longer holds injectAndAssert and injectExpectReject both call refreshTxTimestamp before signing, so every timestamp in a transaction literal was discarded. Beyond the noise it read as meaningful, and once was: Scenario 9.1b set a deliberate offset that was silently reset, and the step passed for the wrong reason until the discrepancy was chased down. fundAccount keeps its timestamp — it builds and signs directly rather than going through either helper, so its value is the one actually used.
- add sc21Tx for the fields every project transaction repeats - add sc21EndorseMilestoneTime for the propose-then-endorse sequence - collapse 34 transaction literals and 5 endorsement loops onto them Each literal restated the network id, the proposal id and the sender's address around the one or two fields the step was actually about. The endorsement sequence appeared six times in two shapes, half of them keying the proposer off an `i === 0` check written out at each call site — the rule that a proposer counts as endorsement #1 now lives in the helper that applies it. Two call sites keep their loops. Milestone 1's start interleaves write-once rejections between the proposal and the endorsements, and termination is a different mechanism: every submission carries a reason and none proposes a time.
- add sc21EarlyPayout and sc21LatePayout, reading the fixture amounts - replace five hardcoded expectations with them The mint expectation already derived from sc21Milestones while every per-milestone assertion restated the same amounts as literals. Changing a fixture amount then meant hand-updating the assertion, which is how the late payout came to convert a pre-subtracted total and drift by a wei. A stale hardcoded expectation also fails misleadingly: it reads as a product bug, and the tempting fix is to edit the number, which quietly changes what the step covers. Each amount is still converted separately, because the handler converts separately — summing or subtracting in USD first truncates once rather than once per amount.
- type the extra fields so type, networkId, from, proposalId and timestamp cannot be passed extra was spread after the invariant fields, so a caller could retarget a transaction at a different proposal or sender without anything saying so. Spreading it first would have made the override silently ignored instead, which is no better; typing those keys as never makes the attempt fail to compile. Verified both directions: the 34 existing call sites still typecheck, and adding proposalId to one of them errors with "Type 'string' is not assignable to type 'never'".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.