Skip to content

canton: revert guardianGovernance fee-pin functional changes (re-review)#45

Merged
bengtlofgren merged 1 commit into
integration/cantonfrom
revert/guardiangovernance-fee-pin
Jul 16, 2026
Merged

canton: revert guardianGovernance fee-pin functional changes (re-review)#45
bengtlofgren merged 1 commit into
integration/cantonfrom
revert/guardiangovernance-fee-pin

Conversation

@bengtlofgren

Copy link
Copy Markdown

Summary

#41 merged directly without going through review. This surgically reverts just its functional changes:

  • The GetGuardianGovernance choice on CoreState.
  • The guardianGovernance field on Emitter/EmitterRegistry/ReplayRootRegistry.
  • The pin assertions at the three fee-charging call sites (PublishMessage, RegisterEmitter, ClaimReplayRoot).
  • The 3 regression tests that proved the fix (they test functionality being removed here).
  • The corresponding doc/README claims describing this security property.

Deliberately does not touch #44 (comment-density cleanup) or anything else merged since — every other line #44 tightened in these same files is untouched. A plain git revert of #41 wasn't usable here since #44 rewrote overlapping comments afterward; this is a hand-picked inverse of just the functional pieces, verified against the original #41 diff file by file.

Why

Not a rejection of the fix — it's a real, already-verified security improvement (closes a shadow-CoreState fee-redirect path). It just went in without the normal review step, and should get one before being final. A follow-up PR (the literal git revert of this commit's squash) will re-propose adding it back, ready for that review.

Test plan

  • dpm build clean on both packages
  • dpm test: 41/41 passing (44 minus the 3 regression tests for now-reverted functionality)
  • Confirmed zero GetGuardianGovernance/functional guardianGovernance-pin remnants anywhere in the tree (grep -rn clean)

https://claude.ai/code/session_01VFr5RtBKysvjdRN2nKmhMV

#41 merged directly without going through PR review. Reverting the
functional changes (the GetGuardianGovernance choice, the guardianGovernance
field on Emitter/EmitterRegistry/ReplayRootRegistry, the pin assertions at
the three fee-charging call sites, and the 3 regression tests proving them)
so the fix can go through proper review this time.

Deliberately does NOT touch #44 (comment cleanup) or anything else merged
since -- only the functional pieces #41 introduced, hand-picked out of the
current tree so #44's independent comment tightening survives untouched in
the same files. Verified: dpm build clean, 41/41 tests passing (44 minus the
3 regression tests testing now-reverted functionality).

A follow-up PR (the literal git revert of this commit) will re-propose
adding this back, for review this time.
@bengtlofgren
bengtlofgren requested a review from evan-gray as a code owner July 16, 2026 12:19
@bengtlofgren
bengtlofgren merged commit a1f225e into integration/canton Jul 16, 2026
24 of 28 checks passed
@bengtlofgren
bengtlofgren deleted the revert/guardiangovernance-fee-pin branch July 16, 2026 12:19
bengtlofgren added a commit that referenced this pull request Jul 16, 2026
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.

1 participant