Skip to content

feat(sns-governance): expose latest reward-event shares on neurons - #11078

Open
aodl wants to merge 2 commits into
dfinity:masterfrom
aodl:agent/expose-sns-reward-event-shares
Open

feat(sns-governance): expose latest reward-event shares on neurons#11078
aodl wants to merge 2 commits into
dfinity:masterfrom
aodl:agent/expose-sns-reward-event-shares

Conversation

@aodl

@aodl aodl commented Aug 8, 2026

Copy link
Copy Markdown

Context -> https://forum.dfinity.org/t/exposing-sns-voting-participation-shares-after-reward-settlement/74916

What

SNS Governance already calculates exact per-neuron reward shares while settling each reward event, but currently discards them after using them to allocate native maturity.

This change stores each participating neuron's positive share together with the existing reward-event end timestamp in a new optional Neuron.latest_reward_event_participation field. The field is exposed through the existing get_neuron and paginated list_neurons APIs.

Participation is recorded independently of the native reward configuration, so it remains available when SNS voting reward rates are zero. Existing maturity allocation, proposal settlement, ballot clearing, reward timing, neuron visibility, and pagination remain unchanged.

Why

This makes the canonical SNS voting-participation weighting available after settlement, allowing generic SNS applications to allocate external, non-inflationary rewards without independently reconstructing ballots.

Semantics and consumer contract

reward_shares is:

The sum of the neuron's canonical ballot voting power over all reward-eligible Yes and No ballots in proposals settled by the identified reward event.

Consequently:

  • Yes and No ballots contribute their recorded voting power.
  • Unspecified ballots contribute nothing.
  • Direct and followed votes are treated identically because following has already produced the canonical ballot.
  • A neuron's voting power is summed across all proposals settled in the event.
  • The value reflects native SNS voting power, not a count of votes.

The value is not:

  • a percentage or normalized score;
  • a maturity amount;
  • an SNS token amount;
  • cumulative across events;
  • per-proposal data;
  • reward-event history.

The field identifies the neuron's most recent event with positive participation, which is not necessarily the globally latest reward event. Consumers must compare:

neuron.latest_reward_event_participation
    .reward_event_end_timestamp_seconds
==
latest_reward_event.end_timestamp_seconds

An absent field or a different timestamp means the neuron has zero shares in the target event.

Only positive participants are updated. A neuron that does not participate in a newer event may retain an older tagged value; the timestamp makes that value unambiguously stale without requiring Governance to rewrite every neuron.

RewardEvent.settled_proposals continues to distinguish:

  • no proposals settled: the list is empty;
  • proposals settled but no eligible votes: the list is nonempty, but no neuron receives positive shares;
  • normal participation: the list is nonempty and positive participants receive tagged shares.

Consumers may normalize the raw shares over whatever eligible neuron set their application defines.

API representation

The Candid shape is equivalent to:

type Uint128 = record {
  high : nat64;
  low : nat64;
};

type RewardEventParticipation = record {
  reward_event_end_timestamp_seconds : nat64;
  reward_shares : opt Uint128;
};

type Neuron = record {
  // Existing fields...
  latest_reward_event_participation :
    opt RewardEventParticipation;
};

The unsigned value is reconstructed as:

(high << 64) | low

u128 is used because one neuron's u64 ballot voting power can be summed over multiple proposals in the same reward event. Protobuf has no native uint128, so the value follows the repository's established high/low representation pattern.

The inner reward_shares is optional because protobuf message fields have presence semantics. Governance always populates it when writing a participation record; consumers should still handle an absent inner value defensively.

Implementation and design decisions
  • Shares are stored directly from the existing neuron_id_to_reward_shares map after it has been fully calculated.
  • The reward-share calculation is not duplicated.
  • Participation is not derived from eventual maturity.
  • The exact timestamp already used for the new RewardEvent is reused as the event tag.
  • Recording occurs before the existing maturity calculation and is not conditional on a positive reward outcome.
  • Existing ordinary versus auto-staked maturity behavior does not affect the participation value.
  • The existing integral-Decimal invariant is checked before exact conversion to u128; negative or fractional values are rejected.
  • No new query endpoint, pagination scheme, participation map, timer, or reward history is added.
  • Old protobuf state decodes with latest_reward_event_participation = None.

When a neuron is split, the parent retains its historical participation and the new child starts with None. The settled event attributed the ballots to the parent's neuron ID; copying the value would double the event total, while dividing it would introduce a new post-settlement redistribution policy rather than exposing the calculation Governance actually made.

Reading a consistent paginated snapshot

A client can safely obtain the latest event's complete participation snapshot using the existing APIs:

  1. Call get_latest_reward_event and record end_timestamp_seconds.
  2. Page through list_neurons with of_principal = None.
  3. Use only participation records tagged with the recorded timestamp.
  4. Call get_latest_reward_event again.
  5. If the timestamp changed during pagination, discard the pages and restart.

Governance does not need to hold a read lock across query calls.

Validation

The following passed locally after implementation:

  • cargo fmt --all -- --check
  • cargo clippy -p ic-sns-governance --all-targets -- -D warnings
  • cargo test -p ic-sns-governance --lib — 328 tests
  • generated protobuf/API file check
  • backward Candid compatibility check
  • production Candid interface equality
  • test-feature Candid interface equality
  • SNS Governance unit tests — 324 tests
  • SNS Governance test-feature unit tests — 328 tests
  • SNS Governance integration tests — 76 tests
  • focused reward-event participation tests — 6 tests
  • complete SNS neuron integration target — 37 tests
  • complete SNS upgrade target — 7 tests
  • //rs/sns/governance/... — all 11 test targets

Coverage includes:

  • direct Yes and No votes;
  • Unspecified ballots;
  • followed voting through the production cascade path;
  • summation across multiple proposals;
  • independent neuron totals;
  • zero native reward rates;
  • unchanged ordinary and auto-staked maturity;
  • ballot clearing and proposal settlement;
  • event replacement without a global clearing pass;
  • no-proposal and no-eligible-vote events;
  • deterministic pagination and get_neuron;
  • exact values above u64::MAX;
  • legacy protobuf decoding;
  • split-neuron semantics;
  • upgrade compatibility.
Related discussions

This addresses a recurring request to retain usable participation information after proposal ballots are cleared:

In addition to the tests within this repository, I've also run a separate SNS project against this release locally and confirmed the desired behaviour against tests within that repo too.

Alexander Lorimer and others added 2 commits August 8, 2026 14:20
Persist each participating neuron's exact reward shares from the existing
reward-settlement calculation, tagged with the reward event end timestamp,
and expose them through the existing neuron query responses.

Participation remains available when native reward rates are zero, while
maturity allocation, proposal settlement, ballot clearing, reward timing,
and neuron pagination remain unchanged.
@aodl
aodl requested a review from a team as a code owner August 8, 2026 16:15
@github-actions github-actions Bot added the feat label Aug 8, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

  5. In the text entry box, respond to each of the numbered items in the previous
    section, declare one of the following:

  • Done.

  • $REASON_WHY_NO_NEED. E.g. for unreleased_changelog.md, "No
    canister behavior changes.", or for item 2, "Existing APIs
    behave as before.".

Brief Guide to "Externally Visible" Changes

"Externally visible behavior change" is very often due to some NEW canister API.

Changes to EXISTING APIs are more likely to be "breaking".

If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.

If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.

Reference(s)

For a more comprehensive checklist, see here.

GOVERNANCE_CHECKLIST_REMINDER_DEDUP

@cla-idx-bot

cla-idx-bot Bot commented Aug 8, 2026

Copy link
Copy Markdown

Dear @aodl,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA.

If you decide to agree with it, please visit this issue and read the instructions there. Once you have signed it, re-trigger the workflow on this PR to see if your code can be merged.

— The DFINITY Foundation

@zeropath-ai

zeropath-ai Bot commented Aug 8, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 97dfffb.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs
Add Uint128 struct and RewardEventParticipation to Neuron, with related doc comments
Enhancement ► rs/sns/governance/canister/governance.did
Extend Neuron with latest_reward_event_participation field; add Uint128 and RewardEventParticipation proto definitions
Enhancement ► rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto
Add Uint128 and RewardEventParticipation messages; extend Neuron with latest_reward_event_participation
Enhancement ► rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs
Add Uint128 and RewardEventParticipation definitions in generated code for Rust proto bindings
Enhancement ► rs/sns/governance/src/governance.rs
Import and utilize Uint128; initialize latest_reward_event_participation in new neuron and governance paths; integrate reward participation handling in creation and updates; introduce helper for reward shares conversion and populate latest_reward_event_participation on events
Enhancement ► rs/sns/governance/src/pb/conversions.rs
Implement conversions between Uint128 and pb_api Uint128; map RewardEventParticipation between internal and API representations
Enhancement ► rs/sns/governance/src/types.rs
Add From for Uint128 and From for u128 conversions; include Uint128 in type imports
Enhancement ► rs/sns/governance/unreleased_changelog.md
Document addition: expose each neuron's exact voting reward shares for its latest participating reward event via get_neuron/list_neurons
Enhancement ► rs/sns/integration_tests/src/neuron.rs
Access and validate latest_reward_event_participation and reward_event_end_timestamp_seconds in rewards test flow

@aodl

aodl commented Aug 8, 2026

Copy link
Copy Markdown
Author

cc @daniel-wong-dfinity-org

@borovan

borovan commented Aug 8, 2026

Copy link
Copy Markdown

Consumer review from ic-query

Thanks for putting this together. This is a strong fit for ic-query's existing SNS reward-checkpoint work: exposing the canonical voting-power weight directly is much better than reconstructing it from ballots or inferring it from maturity changes, and it remains useful when native reward rates are zero.

Main request: add an event-level total and capability marker

Before downstream consumers build on this contract, would you consider adding an optional canonical total to RewardEvent, for example:

optional Uint128 total_reward_shares = <unused tag>;

For every reward event created by a Governance version supporting this feature, it should be populated, including Some(0). Legacy reward events would naturally decode it as absent.

This would solve two important consumer problems:

  1. Rolling-upgrade capability detection. If every neuron's new field is absent, a client currently cannot distinguish an older SNS Governance Wasm from a new event with legitimately zero positive participation. This matters especially because the feature is intended to work when native reward rates are zero.
  2. Snapshot validation and normalization. A paginated consumer could check that the sum of all neuron records tagged with the current event timestamp exactly equals Governance's canonical total. It also supplies an authoritative denominator instead of requiring consumers to normalize over whichever rows they happened to enumerate.

An equivalent event-level summary/capability wrapper would work too; the important properties are explicit feature availability and a canonical total populated even for zero.

Other feedback

  • Please include an explicit human privacy/threat-model review. The field does not reveal Yes versus No, but it does reveal that a neuron participated and its aggregate voting-power magnitude, potentially indefinitely when that neuron does not participate again. That is intrinsic to the use case, but it should be weighed explicitly against the post-settlement ballot-privacy concerns discussed in the community.
  • Define malformed inner-option behavior. The API permits Some(RewardEventParticipation { reward_shares: None }), even though Governance always writes the value. Consumers should treat a matching-event record with missing shares as invalid evidence, not as zero. Similarly, a matching-event value of zero should be invalid if only positive participants are persisted.
  • Add an end-to-end value-above-u64 test. The conversion helper covers u128, but it would be valuable to settle multiple proposals whose voting-power sum exceeds u64::MAX, then verify the exact high/low value through both list_neurons and get_neuron. That protects the full calculation → storage → API path.
  • Record worst-case state/response growth. The per-neuron addition looks modest and reuses the existing reward loop, but a maximum-neuron state-size estimate and representative response-size measurement would make the operational cost explicit.

How ic-query would use it

ic-query already performs strict list_neurons exhaustion bracketed by complete reward-event, nervous-system-parameter, and running-version responses. With this field it can produce direct, single-checkpoint participation evidence:

  • preserve the raw event timestamp and high/low limbs;
  • expose an exact decimal string in JSON rather than a potentially lossy JSON number;
  • classify absent or older tags as zero shares for the bracketed event;
  • reject malformed matching-event records;
  • sum using checked u128 arithmetic and compare with the proposed event-level total;
  • expose exact numerator/denominator values without floating-point percentages.

This would remove the need for two maturity checkpoints, immediate-event maturity reconciliation, and maturity-conversion policy assumptions when the goal is external participation weighting. It still would not infer a payout beneficiary from neuron permissions; beneficiary authorization is a separate application concern.

Overall, the per-neuron implementation looks useful and well targeted. The event-level total/capability marker is the main addition I would want before treating the result as a robust generic allocation contract.

@borovan

borovan commented Aug 8, 2026

Copy link
Copy Markdown

Narrow follow-up on the privacy point above

After re-reading the current patch, I think the concern can be addressed without introducing a new commitment system or substantially expanding this PR.

The important distinction is between:

  1. storing latest_reward_event_participation internally so reward calculations can use it; and
  2. unconditionally projecting that field into the existing public get_neuron and bulk list_neurons responses.

The second part creates a public, neuron-level activity dataset containing both participation and reward-share magnitude. Because neuron IDs are stable, repeated snapshots can be correlated over time. Splitting does not make this a reliable measure of holder-level participation: it mainly makes correlation easier for unsplit neurons than for holders who deliberately reorganize their stake.

My narrow recommendation for this PR is therefore:

  • Keep the internal stable-state field and reward-event accounting.
  • Do not expose the per-neuron participation field through anonymous list_neurons in this PR.
  • Prefer not exposing it through anonymous get_neuron either.
  • Add the event-level total_reward_shares value suggested above, so external tooling has the denominator needed for aggregate reward calculations.
  • Treat selective per-neuron disclosure as a separate API decision.

If per-neuron access is required in this PR, a bounded compromise would be:

  • omit it from list_neurons;
  • return it from get_neuron only when the caller is authorized for that neuron using an existing permission such as ManagePrincipals; and
  • return it only when its event timestamp matches the latest completed reward event, so an old positive record does not become an indefinitely retained activity marker.

That policy would apply uniformly to every neuron, without stake thresholds or holder-specific exceptions.

This also needs a clear scope boundary in the API documentation: this is an SNS reward-accounting feature and should not establish a general governance API pattern. Any equivalent NNS exposure should require its own design, privacy, and governance review.

At minimum, I would add tests proving that:

  • anonymous list_neurons cannot retrieve per-neuron reward participation;
  • unauthorized get_neuron callers cannot retrieve it;
  • authorized access works when supported;
  • stale participation records are redacted; and
  • the event-level total and per-neuron u128 values round-trip correctly.

This seems like the smallest change that preserves the useful accounting work while avoiding a permanent anonymous per-neuron activity feed.

@borovan

borovan commented Aug 8, 2026

Copy link
Copy Markdown

How SNS could become a precedent for NNS

There is no automatic SNS-to-NNS path. A hypothetical progression would require several separate decisions:

  1. The SNS field is accepted as a useful public transparency feature.

  2. Dashboards and tools begin depending on it.

  3. Public per-neuron reward data becomes treated as an established design pattern.

  4. Someone proposes adding the equivalent feature to NNS neurons.

  5. The NNS implementation copies the SNS disclosure model without applying the NNS’s existing private-neuron protections.

  6. Public collectors build a long-term database of NNS reward participation.

That would require a separate NNS code change, review, canister upgrade proposal, and NNS vote. This PR cannot make it happen by itself.

@borovan

borovan commented Aug 8, 2026

Copy link
Copy Markdown

imo fix the privacy issues or it's going to be a problem

@basvandijk basvandijk added the security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

@aodl

aodl commented Aug 8, 2026

Copy link
Copy Markdown
Author

That would require a separate NNS code change, review, canister upgrade proposal, and NNS vote. This PR cannot make it happen by itself.

Yes. That's correct, this is tightly scoped to the SNS framework

imo fix the privacy issues or it's going to be a problem

Please could you articulate your concern regarding privacy?

This PR doesn't change the status quo regarding privacy in any way, but I'd be happy to discuss this with you further if you disagree.

Thanks for your interest.

@aodl

aodl commented Aug 9, 2026

Copy link
Copy Markdown
Author

Dear @aodl,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA.

If you decide to agree with it, please visit this issue and read the instructions there. Once you have signed it, re-trigger the workflow on this PR to see if your code can be merged.

— The DFINITY Foundation

Done

@aodl

aodl commented Aug 9, 2026

Copy link
Copy Markdown
Author

return it from get_neuron only when the caller is authorized for that neuron using an existing permission such as ManagePrincipals;

There is no such permission guard on readability. It's all unconditionally public.

If you'd like the public/private toggle that the NNS has but for SNSs, I would suggest taking that to a new PR. Note that that would actually be a bigger change than you expected. If it were well implemented though, I'd probably support it.

@aodl

aodl commented Aug 9, 2026

Copy link
Copy Markdown
Author

@daniel-wong-dfinity-org, @basvandijk, I'm not sure why the CLA task is still blocking. I actioned the request yesterday

@borovan

borovan commented Aug 9, 2026

Copy link
Copy Markdown

https://forum.dfinity.org/t/beyond-the-sns-why-toko-could-be-its-natural-successor/74992/2 - I think we're going to replace the SNS anyway

@borovan

borovan commented Aug 9, 2026

Copy link
Copy Markdown
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contributor feat security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants