Cip 0105 main

[dfordivam]

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).

[phillip-cashen]

nit: you are already used castAndRound on line 24, so this does nothing.
Also on line 24 you include the withdrawnAmount in the castAndRound, while here (and on line 31) it is excluded, is this intentional and why ? ( i assume some rounding errors could occur)

[phillip-cashen]

Suggested change

	aggregateLockedAmulet : Amulet
	VestingLockedAmulet: Amulet

nit: to keep naming consistent and seperate from the agrgrgateLockedAmulet

[phillip-cashen]

nit: we technical here have the same minor issues as i mentioned for Jonathan.

authorizers [Bob,Alice] against unlockControllerSets [Alice,Bob] will fail (even though they contain the same elements)

[phillip-cashen]

Suggested change

	unlockedAmulet <- create Amulet with
	unlockedAmuletCid <- create Amulet with

nit: (to keep naming consistent with amulet approach)

[phillip-cashen]

Suggested change

	vestedAmulet <- if remainingAggregateLockedAmount > 0.0 then do
	vestedAmuletCid <- if remainingAggregateLockedAmount > 0.0 then do

nit: (to keep naming consistent with amulet approach)

[phillip-cashen]

this is never validated if it is actually open.

you probably need something like:

        round <- fetchReferenceData (ForDso dso) openMiningRoundCid
        require ("mining round is open: " <> show round) (round.opensAt <= now)

this would only work however if the authorizers contains DSO, eles you would have to do something like:

change to ContractId OpenMiningRound instead of round
and then use:

fetchPublicReferenceData (ForDso with dso = expectedDso) openRoundCid (OpenMiningRound_Fetch fetchAs ...)

if going with the second approach we also need to validate that openRound.dso == expectedDso

[phillip-cashen]

i would probably add an ensure here, with checks like:
validExpiringAmount aggregateLockedAmulet.amount
startDate < endDate
withdrawnAmount >= 0.0

[phillip-cashen]

i would probably add some max 0.0 {returnValue} on the two places you return, just to safeguard against transient negatives

[phillip-cashen]

instead of create Amulet i would suggest to create a new non-consuming choice in amulet rules like AmuletRules_VestingWithdrawMint and then add amuletRulesCid as a argument on the class. allow you to easily use existing helpers to validate and add things like meta data etc.

pseudo example snippet:

nonconsuming choice AmuletRules_VestingWithdrawMint : AmuletRules_VestingWithdrawMintResult
  with
    owner : Party
    amount : Decimal
    openRoundCid : ContractId OpenMiningRound
    expectedDso : Optional Party
  controller dso
  do
    checkExpectedDso dso expectedDso
    require "amount > 0" (amount > 0.0)
    openRound <- fetchReferenceData (ForDso with dso) openRoundCid
    now <- getTime
    require "round is open" (openRound.opensAt <= now)
    let cfg = transferConfigAmuletFromOpenRound openRound
    amuletCid <- create Amulet with
      dso
      owner
      amount = expiringAmount cfg.holdingFee amount openRound.round
    return AmuletRules_VestingWithdrawMintResult with
      amuletCid
      meta = Some (simpleHoldingTxMeta TxKind_Unlock (Some "vesting withdraw") None)

[phillip-cashen]

remove the show from the comparison, this weakens it (you can look at checkExpectedDso for inspiration)