fix(contracts): audit remediation, Holacracy compliance, ERC-8004 agents#48
Merged
Merged
Conversation
…ance
Addresses all Critical/High/Medium findings from the security audit plus
architectural improvements informed by ethskills analysis.
Security fixes:
- C-1: RoleRegistry.setGovernanceProcess restricted to factory (OrgFactory)
- H-1: ActionVoting stores orgId at init, uses it for admin checks
- H-2: MeetingComponentsFactory.deploy() gated by org admin check
- H-3: MeetingFactory stores orgId, validates on all public functions
- M-1: adoptProposal enforces zero open objections before adoption
- M-2: removeOrgAdmin prevents last-admin lockout
- M-3: _mintInitialTokens validates array length match
- L-1: GovToken.setMinter(address(0)) reverts with ZeroAddress
- L-5: GovToken emits MinterChanged event on minter transfer
- L-7: Election change type unassigns previous lead before assigning new
Holacracy compliance (§5.3.3-5.3.4):
- resolveObjection requires objector or circle facilitator (not admin)
- setCircleFacilitator(circleId, facilitator) admin-gated setter
- Lead Link / admin cannot unilaterally overrule objections
Gas optimizations:
- _assertOrgAdmin reads storage directly (-24k gas per admin call)
- setRoleRegistryGovernanceProcess uses storage pointer
- Remove redundant zero writes in createOrganization
- Remove no-op status=Draft assignment
- Precompute keccak256('role') as constant
New features:
- ERC-8004 agent identity: linkAgentIdentity(orgId, registry, agentId)
- Proposal expiry: MAX_PROPOSAL_AGE (14 days), permissionless discard
- GovernanceProcessSet event on RoleRegistry
- CircleFacilitatorSet event on MeetingFactory
Tests: 130 passing (was 97), including 33 new tests covering all changes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Summary
Addresses all Critical/High/Medium findings from the security audit, adds gas optimizations, Holacracy-compliant facilitator authority, ERC-8004 agent identity, and proposal expiry.
Security Fixes (Audit Findings)
RoleRegistry.setGovernanceProcessrestricted to OrgFactory — prevents front-running governance hijackActionVotingstoresorgIdat init, uses it for admin checks (was passingcircleIdasorgId)MeetingComponentsFactory.deploy()gated by org admin check — was permissionlessMeetingFactorystoresorgId, validates on all public functions — prevents cross-org escalationadoptProposalenforces zero open objections before adoptionremoveOrgAdminprevents last-admin lockout (tracks_orgAdminCount)_mintInitialTokensvalidatesinitialHolders.length == initialAmounts.lengthGovToken.setMinter(address(0))reverts withZeroAddress()GovTokenemitsMinterChangedevent on minter transferHolacracy Compliance (§5.3.3-5.3.4)
resolveObjectionrequires objector (withdrawal) or circle facilitator (dismissal) — Lead Link/admin cannot unilaterally overrule objectionssetCircleFacilitator(circleId, facilitator)— admin-gated setter (bridge until elections run onchain)New Features
linkAgentIdentity(orgId, agentRegistry, agentId)— org members can link onchain agent NFTsMAX_PROPOSAL_AGE(14 days),discardExpiredProposalis permissionlessGovernanceProcessSet,CircleFacilitatorSet,AgentIdentityLinked,MinterChangedGas Optimizations
_assertOrgAdminreads storage slot directly instead of loading full Organization struct (-24k gas per admin call, 30-42% savings)setRoleRegistryGovernanceProcessusesstoragepointercreateOrganizationkeccak256('role')precomputed as constantBreaking Changes
OrganizationFactoryconstructor now takes 3 params:(roleRegistryImpl, ensRegistrar, meetingComponentsFactory)RoleRegistry.initialize(address _factory)— wasinitialize()MeetingFactory.initialize(uint256 _orgId, ...)— wasinitialize(address, address)ActionVoting.initialize(uint256 _orgId, ...)— wasinitialize(address, address, address)(roleId, newLead, previousLead)— was(roleId, lead)resolveObjectionauth: facilitator, not adminTest plan
wagmi generateafter merge to regenerate TS bindings🤖 Generated with Claude Code