Implement GoodDaoHouses MVP smart contract build#299
Conversation
The deliverible is: the smart-contract build. |
... Addressed in 478c1c7: removed the committed skills payload and replaced the plan-only change with the GoodDaoHouses contract implementation, FlowSplitter integration surface, and focused governance tests. |
Implement GoodDaoHouses MVP smart contract build
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
sirpy
left a comment
There was a problem hiding this comment.
- the fields should be written on chain. for distribution strategy it should be a url to a post on our forum.
- quarterly/term vote: modulo timestamp by termDuration if result is between 0 and votingTermLength then we are in voting period time frame, first vote will "create" the vote record by calling an internal createAlignmentVote, the voteId can simply be the result of timestamp divided by termDuration..
- no need for _activeAlignment or _activeCitizens.
- no need for _voteAlignmentVoters,_voteCitizensVoters, _voteWeightSnapshot assume voting weight for citizen/alignment wont change during vote. check member join date to verify he can participate in current vote.
- totalUnits + totalWeights not required for "vote" record in createAlignmentVote
- voteRecipientWeightedVotes should be divided by BASIS_POINTS
7, finalizeAlignmentVote step is not required. - in executeVote each member units is simply the value he got in voteRecipientWeightedVotes
|
Addressed in 6fb9881. |
|
@copilot fix unresolved code review conversations. make sure fixes are matched by relevant unit tests |
Fixed in the latest commit. Here's what was addressed:
Each fix is accompanied by a new unit test in |
|
@copilot add the suggested tests Currently only the happy path for
Suggested implementation: const cycleStartTime = (await houses.cycleStartTime()).toNumber();
const termDuration = (await houses.termDuration()).toNumber();
const delta =
latestBlock.timestamp < cycleStartTime
? cycleStartTime - latestBlock.timestamp
: (() => {
const offset = (latestBlock.timestamp - cycleStartTime) % termDuration;
return offset === 0 ? 1 : termDuration - offset + 1;
})();
const newCycleStartTime = latestBlock.timestamp + delta;
const validVotingTermLength = Math.floor(termDuration / 2);
// happy-path: admin configures schedule
await expect(
houses
.connect(admin)
.setVotingSchedule(newCycleStartTime, termDuration, validVotingTermLength),
)
.to.emit(houses, "VotingScheduleUpdated")
.withArgs(newCycleStartTime, termDuration, validVotingTermLength);
// negative-path: non-admin / non-committee cannot configure schedule
const { stranger } = await loadFixture(fixture);
await expect(
houses
.connect(stranger)
.setVotingSchedule(newCycleStartTime, termDuration, validVotingTermLength),
).to.be.revertedWithCustomError(houses, "OnlyAdminOrCommittee");
// negative-path: voting term longer than term duration
await expect(
houses
.connect(admin)
.setVotingSchedule(newCycleStartTime, termDuration, termDuration + 1),
).to.be.revertedWithCustomError(houses, "InvalidVotingSchedule");
// negative-path: zero term duration
await expect(
houses
.connect(admin)
.setVotingSchedule(newCycleStartTime, 0, validVotingTermLength),
).to.be.revertedWithCustomError(houses, "InvalidVotingSchedule");
// negative-path: zero voting term length
await expect(
houses
.connect(admin)
.setVotingSchedule(newCycleStartTime, termDuration, 0),
).to.be.revertedWithCustomError(houses, "InvalidVotingSchedule");
// negative-path: cycle start time in the past
await expect(
houses
.connect(admin)
.setVotingSchedule(latestBlock.timestamp - 1, termDuration, validVotingTermLength),
).to.be.revertedWithCustomError(houses, "InvalidVotingSchedule");To make this compile and match your actual contracts/test helpers you’ll likely need to:
@copilot fix and add tests Here To avoid this, either:
uint256 raw = voteRecipientWeightedVotes[voteId][recipient];
require(raw <= type(uint128).max, "Units overflow");
uint128 units = uint128(raw);so that overflow fails loudly instead of corrupting the result. |
... Done in the latest commit. Here's what was added/changed: Contract (
Tests (
|
|
@copilot add the negative test case executeVote: uint128 cast safety |
Done in the latest commit. Since normal voting accumulation can never reach The negative test (
|
Follow-up: add the missing House of Alignment eligibility registryIts missing the HoA pre-registration eligibility registry required by the original plan. Please add a Governance Committee-managed on-chain registry of wallet addresses that are allowed to sign up and stake as House of Alignment members. Requirements:
Suggested shape: struct HoaEligibilityRecord {
bool isEligible;
uint64 listedAt;
uint64 updatedAt;
uint64 delistedAt;
}
function setHoaEligibility(address account, bool isEligible)
external
onlyRole(GOVERNANCE_COMMITTEE_ROLE);
function getHoaEligibility(address account)
external
view
returns (HoaEligibilityRecord memory);Please add tests covering committee-only updates, eligible/ineligible HoA registration through both registration paths, the unchanged Citizens path, and the |
…bility and registration gate
... Done in the latest commit. Here's what was added: Contract (
Tests (
|
| ); | ||
| } | ||
|
|
||
| function stake(uint256 amount) external whenNotPaused { |
There was a problem hiding this comment.
no need for stake only for registerandstake
| ) external override whenNotPaused returns (bool success) { | ||
| require(msg.sender == address(_goodDollar()), "Only G$ token can call"); | ||
|
|
||
| if (_data.length == 0) { |
There was a problem hiding this comment.
no need for only stake. we only use regiseration
| expect(activeAlignmentMembers).to.deep.equal([alignmentOne.address]); | ||
| }); | ||
|
|
||
| it("lets admin or committee set the voting schedule anchor and term lengths", async () => { |
| expect(activeAlignmentMembers).to.deep.equal([alignmentOne.address]); | ||
| }); | ||
|
|
||
| it("lets admin or committee set the voting schedule anchor and term lengths", async () => { |
| address poolAddress; | ||
| } | ||
|
|
||
| struct HoaEligibilityRecord { |
There was a problem hiding this comment.
The hoa eligibility registry is not required.
anyone can stake, but they need to be eventually approved by the committee in approveAlignmentMember
This PR implements the
GoodDaoHousesMVP as an actual smart-contract deliverable, replacing the earlier planning-only artifact. It adds the governance contract, the FlowSplitter integration surface it depends on, a mock for focused testing, and targeted governance tests covering the core membership and vote execution flows.GoodDaoHouses contract
contracts/governance/GoodDaoHouses.sol.Citizens/Alignmenthouse model with per-house minimum stake configuration.transferAndCallregistration, direct register-and-stake, additional staking, approval, revocation, and unstake flows.name/socialLinks, and alignmentname/projectWebpage/missionStatement/distributionStrategy.DAOUpgradeableContract,NameService, access control, pausing, and avatar-authorized upgrades.Membership and voting model
None,Pending,Active,Revoked,Unstaked) and alignment eligibility records onchain.Citizensmembers immediately after staking and gatesAlignmentmembers behind committee-managed eligibility and approval.termDurationandvotingTermLength, where the first ballot cast during the voting window lazily creates the vote record and usestimestamp / termDurationas thevoteId.joinedAtrelative to the current term vote start.voteRecipientWeightedVotes, dividing weighted allocations byBASIS_POINTS.House of Alignment eligibility registry
GOVERNANCE_COMMITTEE_ROLE-managed on-chain pre-registration registry (HoaEligibilityRecord) for wallets allowed to join the House of Alignment.setHoaEligibility(address, bool)grants or revokes eligibility; preserves full timestamp history (listedAt,updatedAt,delistedAt) on add, remove, and re-add operations.getHoaEligibility(address)exposes the full record for audit and client use."Not HoA eligible"via the shared_registerMemberinternal path, so bothregisterAndStakeandtransferAndCallentry points behave identically; Citizens registration is unaffected.HoaEligibilityChanged(address indexed account, bool isEligible)on every registry change.FlowSplitter execution boundary
contracts/governance/IFlowSplitter.solto mirror the pool-based integration surface needed by the MVP.voteRecipientWeightedVotesvalues intoIFlowSplitter.Member[]at the integration boundary.configureFlowSplitter(splitter, poolId)before vote execution; sends zero-unit updates on member removal or unstake.Tests and supporting mock
contracts/mocks/MockFlowSplitter.solfor focused governance testing.contracts/mocks/GoodDaoHousesHarness.sol— a test-only subclass that exposes a committee-gated weight setter used to inject values exceedingtype(uint128).maxfor the overflow safety test.test/governance/GoodDaoHouses.test.ts.uint128cast safety (positive and negative paths).Cleanup
skills-lock.json.Testing
yarn compile(blocked in this sandbox because Hardhat could not reachbinaries.soliditylang.orgto download the Solidity compiler)yarn hardhat test test/governance/GoodDaoHouses.test.ts(blocked for the same compiler-download reason)solccompile check forcontracts/governance/GoodDaoHouses.soltest/governance/GoodDaoHouses.test.tsExample of the implemented execution boundary:
Summary by Sourcery
Introduce the GoodDaoHouses governance contract MVP with member-staking houses, a committee-curated HoA eligibility registry, and FlowSplitter-based vote execution.
New Features:
HoaEligibilityRecord) with full timestamp audit trail; gates new Alignment registrations before any state is written.Enhancements:
Tests: