refactor: improve governance participation logic#538
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the governance participation calculation logic to properly handle orchestrator activation and deactivation cycles, ensuring that only proposals from periods when an orchestrator was active are counted toward their participation rate. The refactoring also improves caching behavior by using cache-and-network fetch policy consistently.
Changes:
- Replaced the generic
transcoderActivatedEventsquery with a specifictranscoderActivationHistoryquery that fetches both activation and deactivation events - Created a new
useGovernanceParticipationhook that encapsulates the logic for calculating governance participation based on activation windows - Refactored
OrchestratingViewcomponent to use the new hook instead of inline query logic
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| queries/transcoderActivationHistory.graphql | New GraphQL query that fetches both activation and deactivation events for a delegate in chronological order |
| queries/transcoderActivatedEvents.graphql | Removed generic query that only fetched activation events with flexible parameters |
| hooks/useGovernanceParticipation.tsx | New hook implementing window-based participation calculation logic with activation/deactivation tracking |
| components/OrchestratingView/index.tsx | Simplified to use new hook, extracted participation calculation to improve readability |
| apollo/subgraph.ts | Updated generated TypeScript types reflecting query changes and added JSDoc formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix participation logic so it works when orchestrators are activated and deactivated. Also improve caching behavoir.
rickstaa
force-pushed
the
refactor-310-participation
branch
from
49365fa to
a400198
Compare
Co-authored-by: copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
rickstaa
force-pushed
the
refactor-310-participation
branch
from
b5e04d4 to
16251e8
Compare
Ensure pending proposals are ommited in the participation calculation.
|
@ECWireless ready for your review. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| round: Number(d.deactivationRound), | ||
| type: "deactivation" as const, | ||
| })), | ||
| ].sort((a, b) => a.round - b.round || (a.type === "deactivation" ? -1 : 1)); |
There was a problem hiding this comment.
The sort tiebreaker at line 49 prioritizes deactivation over activation when both events occur in the same round: (a.type === "deactivation" ? -1 : 1). This means that if a transcoder is both activated and deactivated in the same round, the deactivation is processed first.
This could lead to counterintuitive behavior. If the same round has both an activation and deactivation event:
- With current logic: deactivation comes first, so the activation creates a window starting at that round
- Expected behavior might be: activation comes first, creating a zero-length or instant window
Consider documenting this behavior or revisiting whether this is the correct handling for same-round activation/deactivation events.
| ].sort((a, b) => a.round - b.round || (a.type === "deactivation" ? -1 : 1)); | |
| ].sort((a, b) => { | |
| if (a.round !== b.round) { | |
| return a.round - b.round; | |
| } | |
| if (a.type === b.type) { | |
| return 0; | |
| } | |
| // When in the same round, process activation events before deactivation events | |
| return a.type === "activation" ? -1 : 1; | |
| }); |
There was a problem hiding this comment.
Wondering if this comment sounds reasonable to you @rickstaa . Will ignore if not
There was a problem hiding this comment.
Updated logic. Ignored warning.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
ECWireless
left a comment
ECWireless
left a comment
There was a problem hiding this comment.
Mainly had comments on what the bots had to say
| round: Number(d.deactivationRound), | ||
| type: "deactivation" as const, | ||
| })), | ||
| ].sort((a, b) => a.round - b.round || (a.type === "deactivation" ? -1 : 1)); |
There was a problem hiding this comment.
Wondering if this comment sounds reasonable to you @rickstaa . Will ignore if not
- Sort same-round events with activation before deactivation to avoid dangling windows - Return false from isActiveProposal when currentRoundId is undefined - Filter votes by firstActivationRound to prevent voted exceeding eligible count Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
The calculated total looks wrong. For instance, all orchestrators should be should x out of 39 treasury proposals (at the time of writing this comment), no?
In this PR's preview, I'm getting a different treasury proposal total for each orchestrator page ^^^
Fix participation logic so it works when orchestrators are activated and deactivated. Also improve caching behavoir.