-
Notifications
You must be signed in to change notification settings - Fork 46
CIP-0082 (Daml draft) - 5% development fund impl #3086
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.
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
base: main
Are you sure you want to change the base?
CIP-0082 (Daml draft) - 5% development fund impl #3086
Conversation
meiersi-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Nice work!
wdyt about doing the change wrt where the configs are stored, and then implementing the Daml tests before pinging for another review?
| -- See Splice.Scripts.Parameters for concrete values. | ||
| data DevelopmentFundConfig = DevelopmentFundConfig | ||
| with | ||
| beneficiary : Party |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this design it makes sense to not have to specify a beneficiary at all times. This can be useful to decouple the time when the fund issuance is activated from the time when the collection and funds management rules are setup and implemented.
I'd thus suggest that we make the party optional here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| issueValidatorRewards = True | ||
| executeTransfer rewardsConfig context dso transfer | ||
| configUsd = getValueAsOf now configSchedule | ||
| executeTransfer rewardsConfig context dso configUsd.developmentFundConfig transfer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid repeatedly reading the config. Instead extend:
splice/daml/splice-amulet/daml/Splice/AmuletRules.daml
Lines 760 to 766 in 2fbbe30
| data TransferContextSummary = TransferContextSummary with | |
| featuredAppProvider : Optional Party | |
| config : TransferConfig Amulet | |
| openRound : OpenMiningRound | |
| issuingMiningRounds : Map Round IssuingMiningRound | |
| validatorRights : Map Party (ContractId ValidatorRight) | |
| deriving (Eq, Show) |
Actually, if we put developmentFundBeneficiary into the TransferConfig record, then we already get its value at exactly the right place. That should simplify the code quite a bit.
I'd then put the developmentFundPercentage into the IssuanceConfig where we also define all the tranches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double-check: with this new approach, DevelopmentFundConfig is gone, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| let params = computeIssuingRoundParameters | ||
| miningRound.tickDuration miningRound.amuletPrice miningRound.issuanceConfig summary | ||
| let | ||
| optDevelopmentFundPercentage = (.percentage) <$> (getValueAsOf now configSchedule).developmentFundConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally try to pin all parameters for a round as part of round creation to avoid surprises when issuance configs change.
With my proposal above of putting developmentFundPercentage into IssuanceConfig you get this for free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Are you referring to implementing the full Daml test suite to cover all the new scenarios? |
yes |
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
…ate a DevelopmentFundCoupon contract in AmuletRules_MiningRound_StartIssuing Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
…n from collection and fund management setup Signed-off-by: Jose Velasco - IEU <[email protected]>
…ntFundPercentage and TransferConfig.optDevelopmentFundBeneficiary Signed-off-by: Jose Velasco - IEU <[email protected]>
6569272 to
d3561be
Compare
Signed-off-by: Jose Velasco - IEU <[email protected]>
…igChange test Signed-off-by: Jose Velasco - IEU <[email protected]>
…figChange test Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Signed-off-by: Jose Velasco - IEU <[email protected]>
Done |
📝 PR Summary
This draft PR proposes Daml changes related to CIP-0082 — Establish a 5% Development Fund.
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines