Skip to content

Daml: Fix ClaimExpiredRewards for ValidatorLicense weight #3181

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.

Sign up for GitHub

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

Jump to bottom
Closed
dfordivam wants to merge 10 commits into from
Closed

Daml: Fix ClaimExpiredRewards for ValidatorLicense weight #3181

dfordivam wants to merge 10 commits into from

Conversation

@dfordivam
Copy link
Contributor

@dfordivam dfordivam commented Nov 12, 2025 •
edited
Loading

Fixes : #2902

This contains fixes to DsoRules_ClaimExpiredRewards necessary with the introduction of weight to the ValidatorLicense.

The AmuletRules_ClaimExpiredRewards choice has been deprecated as it was already marked for deprecation.

The unit tests for DsoRules_ClaimExpiredRewards were missing in splice-dso-governance-test, so they have been added

@dfordivam dfordivam marked this pull request as draft November 12, 2025 10:01
@martinflorian-da
Copy link
Contributor

martinflorian-da commented Nov 12, 2025

@dfordivam apologies, I clicked the wrong CI approve button. Closing and reopening your PR in an attempt to fix this...

@martinflorian-da
Copy link
Contributor

martinflorian-da commented Nov 12, 2025

@dfordivam apologies, I clicked the wrong CI approve button. Closing and reopening your PR in an attempt to fix this...

Oh nvm, there is nothing to approve here because your merge target is not splice main... apologies for the confusion...

@dfordivam dfordivam force-pushed the dn/cip-73-daml-claim-expired-reward-fixes branch from 1a27ddd to 2adf6e1 Compare November 13, 2025 06:49
@dfordivam dfordivam marked this pull request as ready for review November 13, 2025 06:52
meiersi-da
meiersi-da reviewed Nov 13, 2025
View reviewed changes
Copy link
Contributor

@meiersi-da meiersi-da left a comment

Choose a reason for hiding this comment

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

Thanks a lot. I'm inclined to go for the fetch-based solution to keep the code change more local. wdyt?

daml/splice-dso-governance/daml/Splice/DsoRules.daml Outdated
Comment on lines 1300 to 1302
expiredValidatorLivenessActivityRecords <- forA (fromOptional [] optValidatorLivenessActivityRecordCids) $ \validatorLivenessActivityRecordCid -> do
expireResult <- exercise validatorLivenessActivityRecordCid ValidatorLivenessActivityRecord_DsoExpire_V2 with closedRoundCid
return expireResult.weight
Copy link
Contributor

@meiersi-da meiersi-da Nov 13, 2025

Choose a reason for hiding this comment

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

I know the original code is alrady duplicated. However, if we go for the fetch solution, then it would be quite nice if we shared that code between AmuletRules and DsoRules.

Copy link
Contributor Author

@dfordivam dfordivam Nov 14, 2025

Choose a reason for hiding this comment

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

The TODO in DsoRules says that AmuletRules should be deprecated, since we are going to update splice-amulet, this may be a good opportunity to do this.

Copy link
Contributor Author

@dfordivam dfordivam Nov 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This code had to make use of fetchUncheckedButArchiveLater, due to the reasons specified in comment.

          -- We need to fetch the weight, and would also prefer doing DsoExpire
          -- We can't do checked fetch as the owner is the validator
          -- hence can't use either fetchAndArchive or fetchUncheckedAndArchive

@dfordivam
Copy link
Contributor Author

dfordivam commented Nov 13, 2025

The static check failed with this error, so I will make fixes to PR then request another CI run

System.IO.IOException: No space left on device : '/home/runner/actions-runner/cached/_diag/Worker_20251113-083526-utc.log' at

@meiersi-da meiersi-da mentioned this pull request Nov 13, 2025
Closed
6 tasks
@meiersi-da
Copy link
Contributor

meiersi-da commented Nov 13, 2025

The static check failed with this error, so I will make fixes to PR then request another CI run

System.IO.IOException: No space left on device : '/home/runner/actions-runner/cached/_diag/Worker_20251113-083526-utc.log' at

Ugh! @isegall-da are we leaking storage?

@isegall-da
Copy link
Contributor

isegall-da commented Nov 13, 2025

The static check failed with this error, so I will make fixes to PR then request another CI run

System.IO.IOException: No space left on device : '/home/runner/actions-runner/cached/_diag/Worker_20251113-083526-utc.log' at

Ugh! @isegall-da are we leaking storage?

I don't think "leaking" per-se, as this is ephemeral storage for each runner, but definitely looks like we're exhausting the storage that we're allocating. I'll take a look.

@isegall-da
Copy link
Contributor

isegall-da commented Nov 13, 2025

@dfordivam you might have just given me a clue to some mysterious CI failures we've been having recently, maybe they're all just "out of disk space" materializing in different ways...

@isegall-da
Copy link
Contributor

isegall-da commented Nov 13, 2025

@dfordivam you might have just given me a clue to some mysterious CI failures we've been having recently, maybe they're all just "out of disk space" materializing in different ways...

The good news is that I found the other issue. The bad news is that it's unrelated to your out-of-space issue. The problem with the latter is that it's on a github-hosted runner, so honestly I'm unsure if and what we can do about that running out of space 🤔

@dfordivam
Daml: Add weight, kind to ValidatorLicense, and SV grant license support
10f85a5 
PR reviewed in hyperledger-labs#2903
CI run hyperledger-labs#3093

Squash commit created by rebasing 5e8d90e7f over f47f017

Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Add unit tests for DsoRules_ClaimExpiredRewards
9d61519 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Sum ValidatorLicense weights in {AmuletRules,DsoRules}_ClaimExpiredRe…
051fd61 
…wards

Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Tests for non-default weights in ClaimExpiredRewards
b2ed843 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Update daml/dars
c033e90 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Deprecate AmuletRules_ClaimExpiredRewards
16c94f0 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Use fetchUncheckedButArchiveLater
a05701b 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Update dars+lock
5612a48 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam dfordivam force-pushed the dn/cip-73-daml-claim-expired-reward-fixes branch from 2adf6e1 to 5612a48 Compare November 14, 2025 07:39
@meiersi-da meiersi-da force-pushed the meiersi/dn/feature-cip-73 branch from 2c829da to 0f126e6 Compare November 14, 2025 15:32
meiersi-da
meiersi-da approved these changes Nov 14, 2025
View reviewed changes
Copy link
Contributor

@meiersi-da meiersi-da left a comment

Choose a reason for hiding this comment

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

Thanks! Looks great!

@dfordivam
Remove 'Dso' from comment as it fails lint check
57bd319 
Signed-off-by: Divam <dfordivam@gmail.com>
@dfordivam
Update dar
18ce465 
Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit to obsidiansystems/canton-splice that referenced this pull request Nov 17, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
4864f05 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in hyperledger-labs#3181
CI run hyperledger-labs#3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
@meiersi-da meiersi-da force-pushed the meiersi/dn/feature-cip-73 branch from 0f126e6 to 4864f05 Compare November 17, 2025 06:57
@meiersi-da
Copy link
Contributor

meiersi-da commented Nov 17, 2025

Closing as this has now been merged into the integration branch

@meiersi-da meiersi-da closed this Nov 17, 2025
dfordivam added a commit to obsidiansystems/canton-splice that referenced this pull request Nov 21, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
a5157b2 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in hyperledger-labs#3181
CI run hyperledger-labs#3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Nov 25, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
9801b32 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Dec 1, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
2f18daf 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Dec 8, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
9712ace 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Dec 9, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
c0a4c09 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Dec 15, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
bf5704d 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
dfordivam added a commit that referenced this pull request Dec 16, 2025
@dfordivam
Daml: Fix DsoRules_ClaimExpiredRewards for ValidatorLicense weight
e6a73a6 
And deprecate AmuletRules_ClaimExpiredRewards

PR reviewed in #3181
CI run #3191

Squashed commit created from 9d61519~18ce4659

Signed-off-by: Divam <dfordivam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@meiersi-da meiersi-da meiersi-da approved these changes

Assignees

@meiersi-da meiersi-da

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dfordivam @martinflorian-da @meiersi-da @isegall-da @ray-roestenburg-da