Cip 0105 basic sv locks by dbzguy56 · Pull Request #6815 · canton-network/splice

dbzguy56 · 2026-08-17T22:34:03Z

WIP, the plan is to have commits be squashed.

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 an upgrade test is required, comment /upgrade_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.
If a logical synchronizer upgrade test is required (from canton-3.5), comment /lsu_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).

meiersi-da

Thanks. Seems a bit too WIP for an in-depth review.

meiersi-da · 2026-08-18T05:55:31Z

      -- in later standards, or on a case-by-case basis between the caller and the
      -- implementer of the interface.
-  deriving (Show, Eq)
+  deriving (Show, Eq, Serializable)


Something is off here. This is part of the commits from staging-0.8.0.

I don't see it at
https://github.com/canton-network/splice/blob/staging-0.8.0/token-standard/splice-api-token-metadata-v1/daml/Splice/Api/Token/MetadataV1.daml#L84
at least; according to @dbzguy56 the compiler was complaining if it wasn't added, but whether that should be happening is a different question.

Yeah that was definitely a mistake, I think there were complaints in other areas. Maybe I mistakenly thought it was there. Reverted

You are right. Explicit serializability is not enabled for that file:https://github.com/canton-network/splice/blob/staging-0.8.0/token-standard/splice-api-token-metadata-v1/daml.yaml, which is fine.

meiersi-da

Thanks.

meiersi-da · 2026-08-19T06:13:15Z

    values : TextMap Text
      -- ^ Key-value pairs of metadata entries.
-  deriving (Eq,Ord, Show)
+  deriving (Eq, Ord, Show)


this change would be good, but we try to avoid interface changes when possible. Let's revert this one.

meiersi-da · 2026-08-19T06:19:15Z

+typeKey : Text
+typeKey = "cip-105/type"
+beneficiaryKey : Text
+beneficiaryKey = "cip-105/lockBeneficiary"
+startDateKey : Text
+startDateKey = "cip-105/startDate"
+endDateKey : Text
+endDateKey = "cip-105/endDate"
+initialAmountKey : Text
+initialAmountKey = "cip-105/initialAmount"
+effectiveAtKey : Text
+effectiveAtKey = "cip-105/effectiveAt"
+
+
+unlockControllerKey : Text
+unlockControllerKey = "cip-105/unlockControllers"
+withdrawControllerKey : Text
+withdrawControllerKey = "cip-105/withdrawControllers"
+substituteControllerKey : Text
+substituteControllerKey = "cip-105/substituteControllers"


nit: please add newlines between the definitions as done in all other files. Also share the prefix and use kebab-case as done for the other metadata keys.

meiersi-da · 2026-08-19T06:19:24Z

+typeKey : Text
+typeKey = "cip-105/type"
+beneficiaryKey : Text
+beneficiaryKey = "cip-105/lockBeneficiary"


adjust to lock-subject

meiersi-da · 2026-08-19T06:19:55Z

 expireLockKey : Text
 expireLockKey = "expire-lock"

-


remove spurious change

Should be done

meiersi-da · 2026-08-19T06:23:41Z

+
+-- TODO: Potentially make return type, it's own data type?
+calculateAvailableWithdrawAmount : Time -> VestingLock -> Decimal -> (Decimal, Decimal)
+calculateAvailableWithdrawAmount currentDateTime (VestingLock {..}) nextIterationFundAmount = max (0.0, 0.0) availAmount


why the max here? Can we inline it into the computation of availAmount?

meiersi-da · 2026-08-19T06:32:05Z

+      && all (\a -> any (a ==) set) actors
+
+
+-- This will also need to get a configuration template to set the vesting period and potentially other settings.


That needs to be passed in via ExternalPartyConfigState.

Added a todo right below it

meiersi-da · 2026-08-19T06:33:30Z

+    checkControllerSpecification arg.actors $ fromOptional (defaultControllerSpecification ownerParty) lock.controllers.unlock
+
+    now <- getFromContextU arg.extraArgs.context effectiveAtKey -- FIXME: read from metadata not context.
+    isLedgerTimeLE now >>= require "effectiveAt must not be in the past"      


use assertWithinDeadline

meiersi-da · 2026-08-19T06:35:01Z

+    let endDate = getEndTime now
+        transferLegId = "unlock"
+
+    let alternateAmount = TM.lookup "cip-105/withdraw-amount" arg.extraArgs.meta.values >>= parseDecimal


I'd suggest that we define types for the choices that we are effectively implementing and use our normal metadata parsing machinery to parse them.

Should be doing this as a TODO within latest PR here: #6894

meiersi-da · 2026-08-19T06:43:22Z

+          controllers = lock.controllers
+
+    -- Not easy to call directly into amulet_allocationFactoryV2_allocateImpl or the like here as that would make a circular dependency.
+    (withdrawTo : ContractId a, _) <- createUnfundedGovernanceAllocation vestingGovernanceLock $ V2.AllocationFactory_Allocate with


@jonored @dbzguy56 : I have the nagging suspicion that we are better off implementing our own templates to represent governance locks, and only using the TSv2 interfaces as a tool to provide a smooth wallet integration. In that case, I'd even go as far as defining internal choices that implement the actual logic, and call them from the interface implementation.

This way we significantly simplify reasoning about the implementation, and avoid polluting the implementation of AmuletAllocationV2 with the concerns of governance locks.

Side note: when I originally proposed to use the "TSv2 allocations" I was thinking about the wallet integration, and less about piggy-backing on AmuletAllocationV2. I was though less clear about it than I am now after having written the high-level implementation spec. Let's dicsuss in our sync.

Discussed in sync

Doing that at #6894 - broadly intending to move features over to there in the "incremental delivery" order unless otherwise requested. As noted this and implementing the V2 interfaces for maximum compatibility does align well with our original intentions for our POC approach.

meiersi-da · 2026-08-19T06:44:47Z

+-- Governance Lock support
+-- =======================
+
+template GovernanceLockProposal with


can we move this out into the GovernanceLock module?

Also: do we even need that? The new spec is based on creating locks directly. This should only be used for substitutions.

I believe you are right, removed this template and portion

canton-network-da assigned isegall-da, martinflorian-da, rautenrieth-da and ray-roestenburg-da Aug 17, 2026

meiersi-da reviewed Aug 18, 2026

View reviewed changes

dbzguy56 force-pushed the cip-0105-basic-sv-locks branch from 2a751c3 to 8d5135f Compare August 18, 2026 17:55

Add initial basic sv locks commit

696ee53

dbzguy56 force-pushed the cip-0105-basic-sv-locks branch from cb1c7f8 to 696ee53 Compare August 18, 2026 21:17

meiersi-da reviewed Aug 19, 2026

View reviewed changes

Make comment suggestions related to CIP-0105

22b03b4

dbzguy56 had a problem deploying to ci-forks August 19, 2026 18:51 — with GitHub Actions Error

Re-add allocation_withdrawImpl case

cd4530d

dbzguy56 had a problem deploying to ci-forks August 19, 2026 20:58 — with GitHub Actions Error

Patch up old tests and to work with current code

d93d9a1

dbzguy56 requested a deployment to ci-forks August 20, 2026 20:57 — with GitHub Actions Waiting

		&& all (\a -> any (a ==) set) actors


		-- This will also need to get a configuration template to set the vesting period and potentially other settings.

Uh oh!

Conversation

dbzguy56 commented Aug 17, 2026

Pull Request Checklist

Cluster Testing

PR Guidelines

Merge Guidelines

Uh oh!

meiersi-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!

Choose a reason for hiding this comment

Uh oh!

meiersi-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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants