Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions daml/splice-amulet/daml/Splice/Amulet.daml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ data FeaturedAppRight_UpdateResult = FeaturedAppRight_UpdateResult with
data FeaturedAppRight_CancelResult = FeaturedAppRight_CancelResult
deriving (Serializable)

data FeaturedAppRight_UpdateUnderlockResult = FeaturedAppRight_UpdateUnderlockResult with
featuredAppRightCid : ContractId FeaturedAppRight
deriving (Serializable)

data AppRewardCoupon_DsoExpireResult = AppRewardCoupon_DsoExpireResult with
featured : Bool
amount : Decimal
Expand Down Expand Up @@ -307,10 +311,18 @@ template FeaturedAppRight with
-- ^ Weight of the this provider's app activity in the computation of traffic-based app rewards.
--
-- If not set, the default weight of 1.0 is used.
requiredLockAmount : Optional Decimal

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

missing: choice to update this amount

I actually realize that we missed an opportunity to make it easy to store additional config parameters on an FA right when we added featured app rights.

Given that we anyways need to add support for instruction contracts to change a featured app right, I suggest to do the following:

-- 1. introduce two new types

data FeaturedAppConfig = FeaturedAppConfig with
  activityWeight : Decimal
  requiredLockAmount : Optional Decimal


data FeaturedAppConfigInternal = FeaturedAppConfigInternal with
  requiredLockAmount : Optional Decimal

-- and use the latter to store a field `config : FeaturedAppConfigInternal` on `FeaturedAppRight`

-- 2. introduce choice to update the current config using `patchable`. The trick is to store the config in an exploded form

choice FeaturedAppRight_UpdateConfig with
  baseConfig : FeaturedAppConfig  
  newConfig : FeaturedAppConfig


-- 3. introduce an instruction for config changes in DsoRules to solve the stable references problem; and prepare for more config being stored on FeaturedAppRights

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That'll have to be config : Optional FeaturedAppConfigInternal, no?

-- ^ Override of the network-wide minimum lock threshold.
-- None means that the default in DsoRulesConfig applies.
firstUnderlockObservedAt : Optional Time

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm inclined to do the join with the grace period at the time where the underlock is observed and not when it is enforced. Something like:

underlockRecoveryDeadline : Optional Time

It has the advantage that an app right can be understood on its own, and we can just add a choice FeaturedApp_EnforceUnderlock that archives the app right.

This also solves the confusion that "first" could refer to the first of all the underlocks, or the first one of the non-recovered underlocks, which actually is the case.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We could do this, but it will mean that if the global grace period length changes, the existing underlock deadlines won't change automatically alongside it. Maybe that's for the best?

I was thinking it's "first" in the sense that if a second SV observes an underlock when one is already present, we don't update it, but maybe that's not worth trying to indicate in the name.

-- ^ Ledger time at which the DSO first observed this provider underlocked.
-- Some t means suspended since time t, and archivable from t + grace period.
-- None means compliant. Never set before enforcement is activated.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Never set before enforcement is activated.

why is that?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm thinking here of the transitional period where we begin to allow FAs to properly re-lock their funds (begins as soon as the DARs go out), but before we begin actually starting the timers. Maybe we could say the grace period is all you get, but my impression was that it ought to be a bit longer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OK, I see. I would probably not note that in the Daml code. The Impl. CIP has this two stage nature built in: https://docs.google.com/document/d/1VGtgNSNHrHnSLggqxlvhCTFrMPgaeCs2OLEJ8Fq9Ar4/edit?tab=t.753u8f9hx8hv#bookmark=id.ck925sjr9rre

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okay, makes sense to me.

where
signatory dso
observer provider
ensure 0.0 <= fromOptional defaultAppActivityWeight activityWeight
&& 0.0 <= fromOptional 0.0 requiredLockAmount

choice FeaturedAppRight_Update : FeaturedAppRight_UpdateResult
with
Expand All @@ -334,6 +346,21 @@ template FeaturedAppRight with
controller provider
do return FeaturedAppRight_CancelResult

choice FeaturedAppRight_UpdateUnderlock : FeaturedAppRight_UpdateUnderlockResult

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
choice FeaturedAppRight_UpdateUnderlock : FeaturedAppRight_UpdateUnderlockResult
choice FeaturedAppRight_UpdateUnderlockStatus : FeaturedAppRight_UpdateUnderlockResult

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Given that you anyways call withdraw directly from DsoRules, I'm wondering whether we shouldn't implement the whole underlock logic in DsoRules, and thus save code.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also consider introducing a data FeaturedAppUnderlockState to make future changes to the underlock logic easy.

with
underlocked : Bool
controller dso
do
now <- getTime
let newObservedAt =
if underlocked
then Some (fromOptional now firstUnderlockObservedAt)
else None
featuredAppRightCid <- create this with
firstUnderlockObservedAt = newObservedAt
return FeaturedAppRight_UpdateUnderlockResult with
featuredAppRightCid

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing: the immediate loss of the ability to create featured app markers if they are underlocked


interface instance Splice.Api.FeaturedAppRightV1.FeaturedAppRight for FeaturedAppRight where
view = Splice.Api.FeaturedAppRightV1.FeaturedAppRightView with dso, provider

Expand Down
4 changes: 3 additions & 1 deletion daml/splice-amulet/daml/Splice/AmuletRules.daml
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,9 @@ template AmuletRules
controller provider
do
require "isDevNet flag is true" isDevNet
featuredAppRightCid <- create FeaturedAppRight with dso; provider; activityWeight
let requiredLockAmount = None
firstUnderlockObservedAt = None
featuredAppRightCid <- create FeaturedAppRight with dso; provider; activityWeight; requiredLockAmount; firstUnderlockObservedAt
return AmuletRules_DevNet_FeatureAppResult with ..

-- Bootstrap the open mining rounds by creating
Expand Down
54 changes: 54 additions & 0 deletions daml/splice-dso-governance/daml/Splice/DsoRules.daml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ data DsoRules_ActionRequiringConfirmation
-- ^ Create BootstrapExternalPartyConfigStateInstruction
| SRARC_UpdateFeaturedAppRight DsoRules_UpdateFeaturedAppRight
-- ^ Update a specific featured app right.
| SRARC_UpdateFeaturedAppUnderlock DsoRules_UpdateFeaturedAppUnderlock
deriving (Eq, Show, Serializable)

data AnsEntryContext_ActionRequiringConfirmation
Expand Down Expand Up @@ -235,6 +236,9 @@ data DsoRules_UpdateFeaturedAppRightResult = DsoRules_UpdateFeaturedAppRightResu
result : FeaturedAppRight_UpdateResult
deriving (Serializable)

data DsoRules_UpdateFeaturedAppUnderlockResult = DsoRules_UpdateFeaturedAppUnderlockResult
deriving (Serializable)

data DsoRules_OnboardValidatorResult = DsoRules_OnboardValidatorResult with
validatorLicense : ContractId ValidatorLicense
deriving (Serializable)
Expand Down Expand Up @@ -543,12 +547,26 @@ data DsoRulesConfig = DsoRulesConfig with
nextScheduledSynchronizerUpgrade: Optional SynchronizerUpgradeSchedule
voteCooldownTime : Optional RelTime -- ^ The minimum time between two votes by the same SV.
nextScheduledLogicalSynchronizerUpgrade: Optional LogicalSynchronizerUpgradeSchedule
faDefaultRequiredLockAmount : Optional Decimal -- ^ The amount of amulet required to be locked by an FA to maintain their FeaturedAppRight if its requiredLockAmount is None.
faUnderlockGracePeriod : Optional RelTime -- ^ The amount of time after which automatic enforcement will withdraw a FeaturedAppRight if its provider remains underlocked.
faUnderlockEnforcementEnabled : Optional Bool -- ^ Whether automatic FA underlock enforcement may now occur, used to support a transitionary period.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This could be a timestamp instead to announce a ledger time

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is coming in 0.8.0: canton-network#6968

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, yeah, I was also thinking about that TextMap from my design doc, but just wanted to put something provisional in place for this exploration.

Comment on lines +550 to +552

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm inclined to push all of these into AmuletConfig under a new record GovernanceLockConfig. They affect amulet users and should thus be available to them when just looking at splice-amulet.

DsoRules then targets the automation of the dso actions on amulet as well as the additional state required to manage the operations and governance of the Global Synchronizer.

deriving (Eq, Show, Serializable)

-- | Read the `voteCooldownTime` from the `DsoRulesConfig` with a default value of 1 minute.
getVoteCooldownTime : DsoRulesConfig -> RelTime
getVoteCooldownTime config = fromOptional (minutes 1) config.voteCooldownTime

-- | Read the `faDefaultRequiredLockAmount` from the `DsoRulesConfig` with a default value of 5_000_000.0
getFaDefaultRequiredLockAmount : DsoRulesConfig -> Decimal
getFaDefaultRequiredLockAmount config = fromOptional 5_000_000.0 config.faDefaultRequiredLockAmount

-- | Read the `faUnderlockGracePeriod` from the `DsoRulesConfig` with a default value of 7 days.
getFaUnderlockGracePeriod : DsoRulesConfig -> RelTime
getFaUnderlockGracePeriod config = fromOptional (days 7) config.faUnderlockGracePeriod

-- | Read the `faUnderlockEnforcementEnabled` from the `DsoRulesConfig` with a default value of False.
getFaUnderlockEnforcementEnabled : DsoRulesConfig -> Bool
getFaUnderlockEnforcementEnabled config = fromOptional False config.faUnderlockEnforcementEnabled

data SynchronizerUpgradeSchedule = SynchronizerUpgradeSchedule with
time : Time
Expand All @@ -575,6 +593,13 @@ data TrafficState = TrafficState with
consumedTraffic: Int -- ^ Bytes of extra traffic consumed before the decentralized synchronizer was bootstrapped.
deriving (Eq, Show, Serializable)

-- | This is an enumeration of the transitions in underlocking state that can be caused by DsoRules_UpdateFeaturedAppUnderlock
data FaUnderlockTransition =
FaUnderlockTransition_Open -- ^ Transition to underlocked state

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure if we want to put a dummy field here to futureproof against SCU

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Depends on what kind of changes we expect. It seems though that we might not need this type at all.

| FaUnderlockTransition_Close -- ^ Transition to compliant state
| FaUnderlockTransition_Enforce -- ^ Withdraw rights
deriving (Eq, Show, Serializable)

template DsoRules with
dso : Party
epoch : Int
Expand Down Expand Up @@ -1072,12 +1097,15 @@ template DsoRules with
with
provider : Party
activityWeight : Optional Decimal
requiredLockAmount : Optional Decimal
controller dso
do
featuredAppRight <- create FeaturedAppRight with
dso
provider
activityWeight
requiredLockAmount
firstUnderlockObservedAt = None
return DsoRules_GrantFeaturedAppRightResult with ..

nonconsuming choice DsoRules_RevokeFeaturedAppRight : DsoRules_RevokeFeaturedAppRightResult
Expand All @@ -1100,6 +1128,28 @@ template DsoRules with
result <- exercise rightCid update
return DsoRules_UpdateFeaturedAppRightResult with result

nonconsuming choice DsoRules_UpdateFeaturedAppUnderlock : DsoRules_UpdateFeaturedAppUnderlockResult
with
rightCid : ContractId FeaturedAppRight

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's a bit of a problem brewing with the approach we are choosing here: the featured app right is referenced by contract-id in DsoRules_UpdateFeaturedAppRight, which in turn is used to execute votes. This only works if concurrent changes to app rights are very seldom.

That will no longer be the case with underlock updates. We have handled this in the past by creating "instruction"-contracts that can then be executed by an SV operator party. See for example https://github.com/canton-network/splice/blob/6e96647fc53583fd3b515c0fb17c1e253364e89e/daml/splice-dso-governance/daml/Splice/DsoRules.daml#L2012-L2020

I suspect that we have to do the same for the featured app rights for both its existing FeaturedAppRight_Update and the new FeaturedAppRight_UpdateUnderlock choice.

transition : FaUnderlockTransition
controller dso
do
right <- fetchChecked (ForDso with dso) rightCid
now <- getTime
require "FA underlock enforcement is active"
(getFaUnderlockEnforcementEnabled this.config)
case transition of
FaUnderlockTransition_Open -> void $ exercise rightCid FeaturedAppRight_UpdateUnderlock with underlocked = True
FaUnderlockTransition_Close -> void $ exercise rightCid FeaturedAppRight_UpdateUnderlock with underlocked = False
FaUnderlockTransition_Enforce -> do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This one doesn't require off-ledger validation. wdyt about moving it into its own choice?

case right.firstUnderlockObservedAt of
None -> fail "FA right is not underlocked"
Some observedAt -> do
require "grace period elapsed"
(observedAt `addRelTime` getFaUnderlockGracePeriod this.config <= now)
void $ exercise rightCid FeaturedAppRight_Withdraw with
reason = "CIP-XXX underlock enforcement"
return DsoRules_UpdateFeaturedAppUnderlockResult

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Generally logic that affects amulet users (e.g., app providers) is captured in AmuletRules and DsoRules only forwards to these choices. It's mostly a code organization principle that we use to ensure that one can reason about amulet related guarantees w/o looking at DsoRules.

I do though have a hard time judging how much extra code that introduces if any at all. If it is a lot, we need to judge whether the change is worth it.

So what I'd suggest is that you first implement the full logic with instructions, and the check how much effort the moving would be.


-- Validator onboarding
-----------------------
Expand Down Expand Up @@ -1868,6 +1918,7 @@ executeActionRequiringConfirmation dso dsoRulesCid amuletRulesCid act = case act
SRARC_GrantFeaturedAppRight choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_RevokeFeaturedAppRight choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_UpdateFeaturedAppRight choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_UpdateFeaturedAppUnderlock choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_SetConfig choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_UpdateSvRewardWeight choiceArg -> void $ exercise dsoRulesCid choiceArg
SRARC_CreateExternalPartyAmuletRules choiceArg -> void $ exercise dsoRulesCid choiceArg
Expand Down Expand Up @@ -2109,6 +2160,9 @@ instance Patchable DsoRulesConfig where
nextScheduledSynchronizerUpgrade = patch new.nextScheduledSynchronizerUpgrade base.nextScheduledSynchronizerUpgrade current.nextScheduledSynchronizerUpgrade
nextScheduledLogicalSynchronizerUpgrade = patch new.nextScheduledLogicalSynchronizerUpgrade base.nextScheduledLogicalSynchronizerUpgrade current.nextScheduledLogicalSynchronizerUpgrade
voteCooldownTime = patch new.voteCooldownTime base.voteCooldownTime current.voteCooldownTime
faDefaultRequiredLockAmount = patch new.faDefaultRequiredLockAmount base.faDefaultRequiredLockAmount current.faDefaultRequiredLockAmount
faUnderlockGracePeriod = patch new.faUnderlockGracePeriod base.faUnderlockGracePeriod current.faUnderlockGracePeriod
faUnderlockEnforcementEnabled = patch new.faUnderlockEnforcementEnabled base.faUnderlockEnforcementEnabled current.faUnderlockEnforcementEnabled

instance Patchable SynchronizerUpgradeSchedule where
patch new base current = SynchronizerUpgradeSchedule with
Expand Down
3 changes: 3 additions & 0 deletions daml/splice-util/daml/Splice/Util.daml
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ instance Patchable Time where
instance Patchable Party where
patch = patchScalar

instance Patchable Bool where
patch = patchScalar

mapDifference : Ord k => Map k a -> Map k a -> Map k k
mapDifference = Map.merge (\_ _ -> None) (\k _ -> Some k) (\_ _ _ -> None)

Expand Down
Loading