Skip to content

feat(evm): collect contract events - #1480

Merged
sebastijankuzner merged 18 commits into
developfrom
feat/evm/contract-events
Sep 30, 2026
Merged

sebastijankuzner merged 18 commits into
developfrom
feat/evm/contract-events

Conversation

@oXtxNt9U

@oXtxNt9U oXtxNt9U commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@codecov

codecov Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.71347% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.04%. Comparing base (df6a785) to head (88b1cd4).

Files with missing lines Patch % Lines
packages/evm/core/src/events.rs 90.90% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1480      +/-   ##
===========================================
+ Coverage    79.38%   80.04%   +0.66%     
===========================================
  Files          953      965      +12     
  Lines        16787    17523     +736     
  Branches      2528     2604      +76     
===========================================
+ Hits         13326    14026     +700     
- Misses        3456     3489      +33     
- Partials         5        8       +3     
Flag Coverage Δ
contracts 91.93% <ø> (?)
packages 79.74% <99.71%> (+0.35%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KovacZan

KovacZan commented Sep 7, 2026

Copy link
Copy Markdown

Voted / Unvoted are not symmetric for vote switches

Flagging this now because the new ContractEvent[] stream is the first surface whose correctness depends on the logs being individually complete, and it's much cheaper to settle the shape before anything builds against it.

The gap. ConsensusV1.vote() handles a switch by calling the internal _unvote(), which emits nothing:

function vote(address addr) external {
    ...
    if (voter.validator != address(0)) {
        _unvote();                       // :300 — internal, emits nothing
    }
    ...
    emit Voted(msg.sender, addr);        // :318
}

function unvote() external {
    emit Unvoted(msg.sender, _unvote()); // :322 — the only `Unvoted` emit site
}

_unvote() (ConsensusV1.sol:636) unlinks the voter, decrements voteBalance / votersCount on the old validator, and deletes the vote — all silently. So a switch A→B produces exactly one log, Voted(voter, B), and no Unvoted(voter, A). The removal from A happened in state but is invisible in the stream. (The other emit Voted at :200 is owner-only addVote, which reverts on AlreadyVoted, so it can't switch.)

Why this is new. The existing consumer is the AccountUpdate fold in collect_dirty_accounts_and_events, which stores final state keyed by voter (account.vote = Some(validator); account.unvote = None) and is written out per-account by api-sync/source/service.ts:315. Last-write-wins per voter is correct no matter what the intermediate logs looked like, so the missing Unvoted was never observable. The ordered per-tx ContractEvent[] now published on block.applied is a log stream, not a fold — different contract with the consumer.

How bad. Not unconditionally broken: Voted carries the voter, so a consumer keeping a voter → validator map can derive the removal itself (look up the voter's current validator, decrement, apply the new one). It breaks for a consumer that maintains only a validator → voters index off the stream, or that reads Voted/Unvoted as symmetric add/remove deltas — which is the natural reading of a pair named that way.

What makes it worth fixing rather than shrugging at is that this PR ships the counter-example: UsernameRegistered(addr, username, previousUsername) (UsernamesV1.sol:12) does carry prior state, so a re-registration is self-describing. Voted doesn't. A consumer will reasonably generalize from one to the other.

Nothing in-repo consumes the stream yet — this PR only adds the block.applied payload — so there's no live bug. Options, in rough order of preference:

  1. Emit Unvoted(msg.sender, oldValidator) before Voted in vote(). Contract change: needs a milestone/upgrade path and changes existing logs.
  2. Add previousValidator to Voted, mirroring UsernameRegistered. Same upgrade cost, symmetric with the username design.
  3. Synthesize the Unvoted in the Rust collector — but collect_dirty_accounts_and_events doesn't have the prior validator either, so this only works if pre-state is threaded in.
  4. Minimum: document on Contracts.Evm.ContractEvent that Voted supersedes any prior vote and that no Unvoted accompanies a switch, so consumers key by voter rather than treating the pair as deltas.

@oXtxNt9U
oXtxNt9U force-pushed the feat/evm/contract-events branch from 149a49f to 5ae2f82 Compare September 25, 2026 04:18
@oXtxNt9U

Copy link
Copy Markdown
Contributor Author

@KovacZan Addressed, thanks. I went with the first option: vote() now emits Unvoted for the previous validator before Voted for the new one, so switching a vote is self-describing in the event stream. The emit moved into _unvote(), which unvote() also uses, so plain unvotes behave as before.

@sebastijankuzner
sebastijankuzner merged commit dbf6db6 into develop Sep 30, 2026
26 checks passed
@sebastijankuzner
sebastijankuzner deleted the feat/evm/contract-events branch September 30, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants