Repository navigation
feat(APP-1099): Implement Safe-backed SPP proposal bodies - #1397
thekidnamedkd wants to merge 130 commits into
Conversation
# Conflicts: # .gitignore # apps/app/package.json # pnpm-lock.yaml
# Conflicts: # apps/app/package.json # pnpm-lock.yaml
milosh86
left a comment
milosh86
left a comment
There was a problem hiding this comment.
Left a couple of comments, but I have more code to check. Sorry, comments might be in a random order and probably all 3 related.
| /> | ||
| </Page.Main> | ||
| <Page.Aside> | ||
| {activeSafeSource != null && ( |
There was a problem hiding this comment.
We need to rethink this change ... GOVERNANCE_MEMBER_PANEL slot now means different thing based on the body type + body type detection is in the page component itself, which mostly defeats the purpose of slots.
One option would be to introduce a new slot for the DaoPluginInfo which Safe plugin can update, and default slot implementation could be like it is now (to cover other plugins).
| featuredDelegates, | ||
| }); | ||
|
|
||
| const { activePlugin: membersPlugin, setActivePlugin: setMembersPlugin } = |
There was a problem hiding this comment.
I'm thinking about something .... does it make sense to add safe as a plugin to a DAO with isBody: true when it is a body in a process in that DAO? ... then we would have a single safe interface type.
Also, in a workspace, a Safe account would always be treated as both process and a body, but this is a bit different than case when safe is a body in a DAO's SPP process.
| const urlParam = searchParams.get(daoMemberListFilterParam); | ||
|
|
||
| const { activePlugin, setActivePlugin, plugins } = | ||
| useDaoPluginFilterUrlParam({ |
There was a problem hiding this comment.
this is also related to previous comment ... if we have safes as real body plugins we wouldn't need to do this complex search and filtering.
btw, if we remove useDaoPluginFilterUrlParam don't we loose tab state persistance?
… plugin-info slot
|
All three at Went with one With Safes as real body plugins the complex search and filtering went away: the SPP-settings scan, the process-list lookup and the synthetic sources are gone, and Members reads ordinary body records. On tab state, removing For the panel there's now a One thing that came with sharing the registry id: SPP stage bodies now resolve their proposal-creation check from the stage condition instead of the resolved registry id, so an external stage reference can't land on the standalone Safe owner + Execute guard. Unsupported-network Safes still go through the external fallback. type-check 9/9, 452 suites / 3200 tests, Biome clean on the changed files. Blocker for merge rather than review: dev returns no |
milosh86
left a comment
milosh86
left a comment
There was a problem hiding this comment.
Hey, I checked the last couple of commits addressing previous comments and it looks good, feels more natural 👍
Now in the main PR I see a bunch of code which seems unrelated to the main goal. I suspect it is now leftover after last refactor which AI still kept. Please double check this, I left some comments but I guess there are more places with the same thing.
| ? featuredDelegatesTabId | ||
| : (membersPlugin?.uniqueId ?? ''); | ||
| const membersPageUrl = `${daoUrl}/members?${daoMembersPageFilterParam}=${membersTabParam}`; | ||
| : searchParams.get(daoDashboardPageMembersFilterParam); |
There was a problem hiding this comment.
Is this change now needed?
|
|
||
| const { activePlugin, setActivePlugin, plugins } = | ||
| useDaoPluginFilterUrlParam({ | ||
| const { data: dao } = useDao({ |
There was a problem hiding this comment.
same here, is this change still needed? It seems this was needed only when safe was not a body plugin?
|
|
||
| if (resolvedDaoId === daoId) { | ||
| return initialParams; | ||
| if (memberSource != null) { |
There was a problem hiding this comment.
check this file too please
| > = async (props) => { | ||
| const { params } = props; | ||
| const { address: rawAddress, addressOrEns, network } = await params; | ||
| const { params, searchParams } = props; |
| IDaoMemberDetailsPageClientProps | ||
| > = (props) => { | ||
| const { address, daoId, network, pluginAddress, tokenAddress } = props; | ||
| const { |
| ); | ||
|
|
||
| if (!plugins.length) { | ||
| const memberSources = daoMemberSourceUtils.resolve({ |
There was a problem hiding this comment.
this file too....please move to another PR if still needed.
This branch was successfully deployed
2 participants
Description
Implements APP-1099. This PR now targets
mainand includes the Safe account work from #1336. Native Safe processes in #1437 stack on this branch.proxy: propose → shared nonce-ordered queue → co-sign → threshold → bundled execution →
report recorded → stage progression.
same-nonce rivals in service order, and the proposal card routes to the account queue while a
live report occupies a slot (and offers no dead deep-link past it).
confirmations stay visible as history, and "approve & re-queue" rebuilds a fresh envelope at a
new nonce without reusing dead signatures.
proposal cards, open an independent payload/hash review, or use the queued timestamp to open
the transaction in Safe. Signing, confirmation and execution stay in proposal context; the
account exposes no removal or replacement actions.
the EIP-712 domain hash, message hash and safeTxHash, matching what the hardware wallet and an
independent tool show.
numeric envelope-field guards; batch-operation recognition; threshold trusted from the Safe
itself; late-report and
maxAdvanceactive/expired semantics; spent read-budget warning;ownership gates and standard wallet + permission dialogs.
safe-accounts.md,safe-spp-signing-trust.md).Backend Safe body/process records, owners and transaction endpoints are now available on dev. The earlier missing-Safe-record blocker no longer applies. Queue cards use
aragonReportsfor proposal navigation, never as execution authority.Verification updated 2026-10-16
0fc944385: transaction 0x2e6360…7e7c. The Safe success event, nonce advancement, executed history and indexed VETO result matched. The proposal displayed Vetoed after reload.bdac781f6fixes Execute overriding the indexing step. A failing-before/passing-after regression and a browser recovery check with the same mined receipt verified indexing through Done, without another transaction.A live shared-Safe/linked-DAO fixture remains unavailable; identity separation, member selection and signer links have deterministic coverage. Backend freshness questions remain, but these checks found no missing backend feature blocking the ticket. Review approval and the existing review threads are still open.
Type of Change
Developer Checklist:
Review Checklist: