-
Notifications
You must be signed in to change notification settings - Fork 17
refactor: improve governance participation logic #538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a400198
16251e8
836e9fd
14cfab2
6197c28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,9 +4,7 @@ import { Box, Flex, Link as A, Text } from "@livepeer/design-system"; | |||||||||||||||||||||||||||||||||||
| import { ArrowTopRightIcon, CheckIcon, Cross1Icon } from "@modulz/radix-icons"; | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| AccountQueryResult, | ||||||||||||||||||||||||||||||||||||
| OrderDirection, | ||||||||||||||||||||||||||||||||||||
| TranscoderActivatedEvent_OrderBy, | ||||||||||||||||||||||||||||||||||||
| useTranscoderActivatedEventsQuery, | ||||||||||||||||||||||||||||||||||||
| useTranscoderActivationHistoryQuery, | ||||||||||||||||||||||||||||||||||||
| useTreasuryProposalsQuery, | ||||||||||||||||||||||||||||||||||||
| useTreasuryVotesQuery, | ||||||||||||||||||||||||||||||||||||
| } from "apollo"; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -32,56 +30,128 @@ interface Props { | |||||||||||||||||||||||||||||||||||
| isActive: boolean; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const Index = ({ currentRound, transcoder, isActive }: Props) => { | ||||||||||||||||||||||||||||||||||||
| const callsMade = useMemo( | ||||||||||||||||||||||||||||||||||||
| () => transcoder?.pools?.filter((r) => r.rewardTokens != null)?.length ?? 0, | ||||||||||||||||||||||||||||||||||||
| [transcoder?.pools] | ||||||||||||||||||||||||||||||||||||
| type ActivationWindow = { start: number; end: number }; | ||||||||||||||||||||||||||||||||||||
| type Participation = { voted: number; eligible: number }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const buildActiveWindows = ( | ||||||||||||||||||||||||||||||||||||
| activations: { activationRound: string }[], | ||||||||||||||||||||||||||||||||||||
| deactivations: { deactivationRound: string }[] | ||||||||||||||||||||||||||||||||||||
| ): ActivationWindow[] => { | ||||||||||||||||||||||||||||||||||||
| const timeline = [ | ||||||||||||||||||||||||||||||||||||
| ...activations.map((a) => ({ | ||||||||||||||||||||||||||||||||||||
| round: Number(a.activationRound), | ||||||||||||||||||||||||||||||||||||
| type: "activation" as const, | ||||||||||||||||||||||||||||||||||||
| })), | ||||||||||||||||||||||||||||||||||||
| ...deactivations.map((d) => ({ | ||||||||||||||||||||||||||||||||||||
| round: Number(d.deactivationRound), | ||||||||||||||||||||||||||||||||||||
| type: "deactivation" as const, | ||||||||||||||||||||||||||||||||||||
| })), | ||||||||||||||||||||||||||||||||||||
| ].sort((a, b) => a.round - b.round || (a.type === "deactivation" ? -1 : 1)); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const windows: ActivationWindow[] = []; | ||||||||||||||||||||||||||||||||||||
| let start: number | null = null; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| for (const { type, round } of timeline) { | ||||||||||||||||||||||||||||||||||||
| if (type === "activation") { | ||||||||||||||||||||||||||||||||||||
| if (start === null) { | ||||||||||||||||||||||||||||||||||||
| start = round; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| start = round; | |
| start = round; | |
| } else { | |
| // Unexpected duplicate activation while a window is already active. | |
| // This is ignored to preserve existing behavior, but logged for visibility. | |
| // If the activation/deactivation protocol guarantees no duplicates, this | |
| // may indicate a data quality issue upstream. | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| "buildActiveWindows: ignoring duplicate activation event", | |
| { activeStartRound: start, activationRound: round } | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if the protocol allows duplicate activations @rickstaa so not sure if this suggestion makes sense
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The buildActiveWindows function may not correctly handle deactivation events that occur when start === null (i.e., before any activation). The condition on line 59 checks start !== null && round >= start, so a deactivation event with start === null is simply ignored.
While this might be the intended behavior (deactivation without activation is meaningless), it could silently hide data integrity issues in the underlying events. Consider adding validation or a warning when encountering a deactivation without a prior activation.
| } else if (start !== null && round >= start) { | |
| } else if (type === "deactivation") { | |
| if (start === null) { | |
| console.warn( | |
| "buildActiveWindows: received deactivation without a prior activation", | |
| { deactivationRound: round } | |
| ); | |
| continue; | |
| } | |
| if (round < start) { | |
| console.warn( | |
| "buildActiveWindows: received deactivation before the current activation start", | |
| { activationStartRound: start, deactivationRound: round } | |
| ); | |
| continue; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it's hard to tell if this is intended since I'm not sure exactly is expected from the protocol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is at least worth sanity-checking. Right now, I can't see a reason why we should default to true
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isActiveProposal function returns true when currentRoundId is undefined (line 74). This means all proposals are considered active if the current round ID is not provided. While this might be intentional fallback behavior, it could lead to incorrect participation calculations if the current round ID is unexpectedly missing.
Consider documenting this behavior or making it more explicit with a comment, especially since this affects how "eligible" proposals are counted.
| const isActiveProposal = (voteStart: string, currentRoundId?: string) => | |
| currentRoundId ? Number(voteStart) <= Number(currentRoundId) : true; | |
| const isActiveProposal = (voteStart: string, currentRoundId?: string) => { | |
| // If currentRoundId is not provided, treat all proposals as active. | |
| // This affects how "eligible" proposals are counted in participation metrics. | |
| if (currentRoundId === undefined) { | |
| return true; | |
| } | |
| return Number(voteStart) <= Number(currentRoundId); | |
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with this, since I'm finding the logic difficult to follow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggestion of Number(vote.proposal.voteStart) >= Number(firstActivationRound) && on line 131 seems reasonable to me
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| query transcoderActivationHistory($delegate: String!) { | ||
| transcoderActivatedEvents( | ||
| where: { delegate: $delegate } | ||
| orderBy: activationRound | ||
| orderDirection: asc | ||
| ) { | ||
| activationRound | ||
| } | ||
| transcoderDeactivatedEvents( | ||
| where: { delegate: $delegate } | ||
| orderBy: deactivationRound | ||
| orderDirection: asc | ||
| ) { | ||
| deactivationRound | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
Consider documenting this behavior or revisiting whether this is the correct handling for same-round activation/deactivation events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if this comment sounds reasonable to you @rickstaa . Will ignore if not