[ci] Zero balance related fields in round aggregates so that they always agree (BFT) #3204

ray-roestenburg-da · 2025-11-14T09:49:02Z

Fixes https://github.com/DACH-NY/canton-network-internal/issues/2493 (all SVs will agree on zero values)
API's that depend on the balance values have already been deprecated and replaced by instrument total supply and holdings summary.

balance values are:

changeToInitialAmountAsOfRoundZero
changeToHoldingFeesRate
cumulativeChangeToInitialAmountAsOfRoundZero
cumulativeChangeToHoldingFeesRate
totalAmuletBalance

Pull Request Checklist

Cluster Testing

If a cluster test is required, comment /cluster_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
If a hard-migration test is required (from the latest release), comment /hdm_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.

PR Guidelines

Include any change that might be observable by our partners or affect their deployment in the release notes.
Specify fixed issues with Fixes #n, and mention issues worked on using #n
Include a screenshot for frontend-related PRs - see README or use your favorite screenshot tool

Merge Guidelines

Make the git commit message look sensible when squash-merging on GitHub (most likely: just copy your PR description).

…they always agree. Signed-off-by: Raymond Roestenburg <[email protected]>

martinflorian-da · 2025-11-14T10:45:25Z

Fixes https://github.com/DACH-NY/canton-network-internal/issues/2493

Can we test this somehow?

Should we also mention this in the release notes?

ray-roestenburg-da · 2025-11-14T10:59:05Z

@martinflorian-da the existing tests for scan aggregation should suffice but will have another look.
I'll also add release notes

martinflorian-da · 2025-11-14T11:02:30Z

the existing tests for scan aggregation should suffice

@ray-roestenburg-da Well if they did they should be failing on main now :)

Specifically I wonder if we can have a test that replicates the current situation on MainNet, and fails without these changes.

(I'm not super confident in my context here.)

ray-roestenburg-da · 2025-11-14T11:05:34Z

Well if they did they should be failing on main now :)

Since all results are zero for these values, they will always agree, so the tests should not fail (that use this code I changed, though possibly there is no test). In any case I'll add a test.

ray-roestenburg-da · 2025-11-14T12:17:42Z

@martinflorian-da @OriolMunoz-da added a test and updated release notes, please have another look.

Also added release notes. Signed-off-by: Raymond Roestenburg <[email protected]>

martinflorian-da

Looks reasonable, thank you! You might want to wait for someone more knowledgeable than me to approve as well though.

martinflorian-da · 2025-11-14T13:35:56Z

docs/src/release_notes.rst

+
+  - The round-based aggregates for balance values (changes to holding fees and initial amounts since round zero)
+    have diverged between scans because of how amulets expire in the ``scan_txlog_store``.
+    The balance values recorded in the round aggregates are effectively not used anymore, and are now set to zero to avoid consensus problems when an SV reads aggregates


not used anymore

By splice apps?

I'm not sure this makes sense, but do some people implement their own variant of BFT scan reads? This should probably be fixes as well then? (I.e., do we need to call this out?)

I'll rephrase it

By scan APIs, I'll rephrase

rephrased. Oh and no, this is not something people implement by themselves, this is for backfilling and starting from known state for the aggregation. When an SV starts and has no aggregates yet, scan will request the totals from the other SVs in BFT fashion, and continue from that round. It's really all internal API to the scan app.

martinflorian-da · 2025-11-14T13:36:33Z

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

      result shouldBe Some(roundAggregate)
    }

+    "get BFT round aggregates from scans that report having the round aggregate ignoring balance fields" in {


sentence is a bit hard to parse

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

martinflorian-da · 2025-11-14T13:44:25Z

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

+      val result = con.getRoundAggregate(round).futureValue
+      result shouldBe Some(roundAggregateZeroBalanceValues)
+
+      // not using the decoder should fail on the randomized balance values.


Why even test/commit this? That's a code path that doesn't exist like this anymore, isn't it?

Just to be sure the decode stuff does something

Well if it didn't the previous should should fail? But I don't mind keeping it

yeah true, I could remove it, but liked to see it was failing the consensus, so I didn't do some encode/decode that did nothing

OriolMunoz-da

There was also this comment by nicu in the original PR:
https://github.com/DACH-NY/canton-network-internal/issues/2493#issuecomment-3472570660
will this be handled in a later PR?

docs/src/release_notes.rst

OriolMunoz-da · 2025-11-14T13:47:04Z

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

+        cumulativeChangeToHoldingFeesRate = randomValue,
+        totalAmuletBalance = randomValue,
+      )
+      def encodeRoundTotals(rt: RoundTotals) = {


afaict this is duplicate with HttpScanHandler's listRoundTotals, DRY

OriolMunoz-da · 2025-11-14T13:47:31Z

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

+          totalAmuletBalance = Codec.encode(rt.totalAmuletBalance),
+        )
+      }
+      def encodeRoundPartyTotals(rpt: RoundPartyTotals) = {


afaict this is duplicate with HttpScanHandler's listRoundPartyTotals, DRY

oh right I'll cleanup thx

...test/scala/org/lfdecentralizedtrust/splice/scan/admin/api/client/BftScanConnectionTest.scala

Signed-off-by: Raymond Roestenburg <[email protected]>

ray-roestenburg-da · 2025-11-14T14:28:32Z

Also added #3206 thanks @OriolMunoz-da for the heads up

Co-authored-by: Oriol Muñoz <[email protected]> Signed-off-by: Raymond Roestenburg <[email protected]> Signed-off-by: Raymond Roestenburg <[email protected]>

[ci] Zero holding fee and initial amount values in responses so that …

029401d

…they always agree. Signed-off-by: Raymond Roestenburg <[email protected]>

ray-roestenburg-da requested review from isegall-da and martinflorian-da November 14, 2025 10:26

ray-roestenburg-da added the sprint2545 label Nov 14, 2025

ray-roestenburg-da requested review from OriolMunoz-da and rautenrieth-da November 14, 2025 10:53

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch from 3221ec5 to 5af39a1 Compare November 14, 2025 12:17

[ci] Added a test for zeroed balance values in bft scan connection.

23d08e3

Also added release notes. Signed-off-by: Raymond Roestenburg <[email protected]>

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch from 5af39a1 to 23d08e3 Compare November 14, 2025 12:19

ray-roestenburg-da self-assigned this Nov 14, 2025

martinflorian-da approved these changes Nov 14, 2025

View reviewed changes

OriolMunoz-da reviewed Nov 14, 2025

View reviewed changes

[ci] review, DRY.

35b6cf6

Signed-off-by: Raymond Roestenburg <[email protected]>

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch from 297f280 to 35b6cf6 Compare November 14, 2025 14:23

OriolMunoz-da approved these changes Nov 14, 2025

View reviewed changes

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch from 6ff61c8 to 3b7357e Compare November 14, 2025 14:25

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch 2 times, most recently from 00b5587 to 00e9d21 Compare November 14, 2025 15:02

[ci] Update docs/src/release_notes.rst

be5e147

Co-authored-by: Oriol Muñoz <[email protected]> Signed-off-by: Raymond Roestenburg <[email protected]> Signed-off-by: Raymond Roestenburg <[email protected]>

ray-roestenburg-da force-pushed the ray/scan_aggr_ignore_bal branch from 00e9d21 to be5e147 Compare November 14, 2025 15:38

ray-roestenburg-da enabled auto-merge (squash) November 14, 2025 15:43

ray-roestenburg-da merged commit 46c0f58 into main Nov 14, 2025
62 checks passed

ray-roestenburg-da deleted the ray/scan_aggr_ignore_bal branch November 14, 2025 16:10

[ci] Zero balance related fields in round aggregates so that they always agree (BFT) #3204

[ci] Zero balance related fields in round aggregates so that they always agree (BFT) #3204

Uh oh!

Conversation

ray-roestenburg-da commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Cluster Testing

PR Guidelines

Merge Guidelines

Uh oh!

martinflorian-da commented Nov 14, 2025

Uh oh!

ray-roestenburg-da commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martinflorian-da commented Nov 14, 2025

Uh oh!

ray-roestenburg-da commented Nov 14, 2025

Uh oh!

ray-roestenburg-da commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martinflorian-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ray-roestenburg-da Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ray-roestenburg-da Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OriolMunoz-da left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ray-roestenburg-da commented Nov 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ray-roestenburg-da commented Nov 14, 2025 •

edited

Loading

ray-roestenburg-da commented Nov 14, 2025 •

edited

Loading

ray-roestenburg-da commented Nov 14, 2025 •

edited

Loading

ray-roestenburg-da Nov 14, 2025 •

edited

Loading

ray-roestenburg-da Nov 14, 2025 •

edited

Loading