Various things to improve in the finance cordapp

(Mike Hearn) #1

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.move has a confusing name that’s inconsistent with the withNewOwner method in the superclass. It should be renamed to be something like withNewOwnerAndAmount.
  • FungibleAsset.Commands just 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.exitKeys exists 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.
  • The FungibleAsset.ExitCommand interface 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.

(richard) #2

minor comment/question from me: it’s not obvious why we’d expect the issuer to sign an Exit transaction. If I want to relinquish a claim against an issuer (presumably in exchange for their making an equivalent amount of value appear somewhere else, I can see why I should be required to sign in all cases. But why them?

(Mike Hearn) #3

Central bank accepts a movement of USD into a special Corda-linked RTGS account.

Recipient gets the money on ledger. It gets traded around for a while.

Someone decides for unclear reasons that he’d prefer to be poor, deletes the money from the ledger. CB doesn’t know.

Now the RTGS has an account problem. There will be money stuck in the CB account “forever”. This will eventually lead the issuer astray as they realise they could maybe go fractional reserve on that …

(richard) #4

hmm… I agree it would be strange not to tell the issuer you’d voluntarily relinquished a claim on them. But we don’t try to solve that problem anywhere else in the codebase. And why not the opposite? i.e. why allow the issuer to cancel its debt to me without my countersigning or necessarily getting to hear about it? Sure… you could say they’re free to renege and let me sue them but it seems like the default check we encourage people to make (check the issuer is ok with owing less to somebody!) is a weird choice. Not a biggie - just a little confusing.

(Mike Hearn) #5

Right, we don’t really know how decentralised fiat cash will work as nobody has tried to do it before, so we’re really just guessing here.

On one hand, perhaps the answer should be to prefer simplicity first, rather than trying to intuit requirements that may not exist. That way the code is easier to read and understand, especially for contracts people will learn from and copy like cash.

On the other hand, the point of ‘smart’ contract logic is to try and ensure everyone has the right view at the right time and that others can check that they had the right view at the right time, so that suggests trying to avoid weird situations like someone unilaterally cancelling obligations an issuer still thinks they have (which can only ever be a mistake, right?). And yes the opposite too - the issuer exits money without the holder of the asset knowing! That should be hard to do anyway, the issuer would need to know the StateRef of the thing they were trying to unilaterally exit, but we should exclude it via signing anyway.

This takes us back to the old discussions we had last year on the topic. For instance does signing an exit mean “I agree to this” or does it merely mean “I disagree that this is legally correct but am aware of the change and am signing to reflect my awareness”.

(richard) #6

agree with all of that… so if we’re worried about being didactic, change exit to require a signature from both and include a comment explaining why somebody MIGHT want to override that behaviour.

(Konstantinos Chalkias) #7

Some of Mike’s suggestions have been addressed here: FungibleAsset cleanup first round.
This is the current form of FungibleAsset:

interface FungibleAsset<T : Any> : OwnableState {
    val amount: Amount<Issued<T>>
    val exitKeys: Collection<PublicKey>
    fun withNewOwnerAndAmount(newAmount: Amount<Issued<T>>, newOwner: AbstractParty): FungibleAsset<T>

There are still two major decisions to be taken:

  • Amount<Issued<T>> Vs Amount<T>

  • retain or remove exitKeys