Whilst we won’t be shipping a stabilised finance CorDapp in v1.0 (only the general non-finance-specific stuff will be committed to as part of that release), we should really finish it off and commit to it soon after.
Here are some notes on things we can improve about it before release.
The finance types are quite complicated for what they do. In the beginning I wrote the code to be quite generic, but as time has gone by, lots of features have ended up being used in quite predictable ways and it seems likely we can simplify. A few ideas:
- We don’t need IssueCommand anymore now privacy nonces are introduced (Andrius is already cleaning this up).
- It’s not obvious to me we need the MoveCommand interface either. Do we really need contract-specific move types? Moreover, if one or two use cases do, can we just use sub-classing instead of an interface, or just require both commands to be present? Can we provide a pre-canned singleton object for move commands? Why make app devs write their own in the common case where it’s only really the presence of a string (in the tx) that matters?
FungibleAsset.movehas a confusing name that’s inconsistent with the
withNewOwnermethod in the superclass. It should be renamed to be something like
FungibleAsset.Commandsjust defines even more interfaces (of which Move is not documented). Apropos the above do we need these? Can we just erase them? If we erase them, a bunch of methods that are there to allow people to create their own sub-classes can also be erased.
- OnLedgerAsset is the contract class that’s paired with FungibleAsset state classes. But their names aren’t related, FungibleAsset is in core and OnLedgerAsset is in finance, and quite a few of the abstract methods on OnLedgerAsset are there only to support the ability to have app-specific move/issue/exit types. If we take out that ability, then we can simplify yet more stuff.
- The amount field on FungibleAsset is not documented, although it’s fairly obvious what it’s for.
FungibleAsset.exitKeysexists mostly to allow asset contracts to customise who has to sign for an exit to be valid. The theory being that a cash contract will need the issuer to sign off on deletion of money from the ledger. But:
- Are there really any cases where the issuer would not be involved in an exit? Consumption of the asset somehow, perhaps?
- If we erase this, can we just push the extra check into a one-liner in the verify function? We’re attempting to be very generic here (you can have a potentially huge number of exit keys by this interface), so that clauses can be the One True Way to write contracts, but if we take clauses out or encourage mixed approaches, I’m not sure what this API complication is buying us. Cash contract can easily test for the issuer key being in the exit command too.
FungibleAsset.ExitCommandinterface has an “amount” field, which would then have to be checked against the amount actually exited. Do we need this? If we have a single constant/singleton exit command, can’t we just assume that the amount can fall by any value (as long as it doesn’t go negative)? Why do we have to repeat this here?
- Obligation has a type parameter P but the doc is a bit confusing, and there is no type bound on P to help the reader understand what kind of thing it’s meant to be.
- Obligation has some extension functions that could apparently be on the class itself?
This is not a comprehensive review.
None of these changes are large or significant, they’re mostly just about removing genericism that experience has shown us isn’t buying us a whole lot.