hyperledger-labs / fabric-token-sdk

The Fabric Token SDK is a set of API and services that lets developers create token-based distributed application on Hyperledger Fabric.
Apache License 2.0
84 stars 60 forks source link

Streamline token chaincode params initialization #743

Open arner opened 2 weeks ago

arner commented 2 weeks ago

Current approach

The Fabric chaincode needs the fabric --init-required functionality: when deploying, before any other transaction, one party has to call the init function which then stores the public params on the ledger.

This is not the most ergonomic feature, because:

  1. Low support by tooling (notably Fabric Smart Client - but also the Fabric Ansible Scripts barely support it)
  2. It forces a deployment model that mixes infrastructure with application, especially if you use chaincode as a service (back and forth between fabric and configuration of the application)
  3. Not so lenient for mistakes (have to deploy a new version of the chaincode, with a new chaincode id, update config, etc, which requires Fabric knowledge).

From the Fabric docs:

Note that in most scenarios it is recommended to embed initialization logic into chaincode rather than use the chaincode lifecycle mechanism described above. Chaincode functions often perform checks against existing state, and initialization state can be implemented like any other chaincode state and be checked in subsequent chaincode function calls. Handling initialization state within chaincode logic rather than with the chaincode lifecycle mechanism has the benefit that you are not limited to a single initialization function, rather you are in full control of initialization logic and can call your own functions that initialize state from an application consistent with how all other application functions are called.

The flow of the init function is as follows:

Init -> ReadParamsFromFile -> if empty use builtInParams -> write to ledger

And when doing a transaction:

  1. Invoke
  2. GetValidator
  3. cc.Initialize
  4. Get params from memory. If not in memory: ReadParamsFromFile -> if empty use builtInParams
  5. Validate
  6. Store actions
  7. AddPublicParamsDependency - adding a Read for the params on the ledger without using them.

As far as I can tell this means that if an endorser changes their public params file and restarts the chaincode, transactions will fail because they're the only one reading it from the file.

Proposal

If you agree with the proposal I can make a PR.

adecaro commented 2 weeks ago

Hi @arner , thanks for reporting this issue. I have a question: Who should be able to perform the update? Should the chaincode enforce some access control on the creator of the proposal?

arner commented 2 weeks ago

As far as I understand, we currently don't have that. Even though we require Init, and don't have a function for updates, we can't stop a quorum of endorsers from crafting a rwset offline, signing it and getting it committed. In my view, if that possibility is there, any hurdle in the chaincode is just cosmetic. In my proposal there is already a conscious decision by the endorsers to do the update, because they each have to manually update the parameters in their chaincode before someone invokes the update transaction (similar to the current way but easier). Contrary to, for instance, approving a transfer, which is done automatically by the chaincode and has no human oversight.

If we want to enforce access control on who initiates the invoke, maybe an option could be key-level endorsement: that can be set to be stricter than the normal endorsement policy. Whether to use it and how to configure it would be different per use case though.

adecaro commented 2 weeks ago

so, the endorsement policy is the trust assumption that covers also against a subset of malicious endorsers that come together and try to update the namespace without an actual request from a user. So, it must set properly to avoid also this.

In this sense, we don't have the issue you point to.

I agree anyway that the process is not the simplest. Unfortunately, it would not be efficient to retrieve for each request the state from the ledger. We must cache it in some way.

So, to summarize, I find it risky to allow anyone to submit new public parameters. The honest endorsers would have no way to check if they must accept these params or not.

But what if we say that the issuer can update the public params? Would this be fine? We can ask the token-sdk to check such a signature. Wdyt?

arner commented 2 weeks ago

so, the endorsement policy is the trust assumption that covers also against a subset of malicious endorsers that come together and try to update the namespace without an actual request from a user. So, it must set properly to avoid also this. In this sense, we don't have the issue you point to.

Agree. It means that (currently/by default) the rules are the same to update any key on the ledger. So a subset of malicious endorsers could arbitrarily reassign ownership of tokens, or change the issuer in the public parameters. I guess this is ok because both are pretty catastrophic, so we'd ensure a strong policy anyway.

I agree anyway that the process is not the simplest. Unfortunately, it would not be efficient to retrieve for each request the state from the ledger. We must cache it in some way.

This is fine. For the caching, the main thing I propose is to change the function behind initOnce (https://github.com/hyperledger-labs/fabric-token-sdk/blob/2e556dd0f1dd48252e6874753e33e5be7947eae1/token/services/network/fabric/tcc/tcc.go#L163) to retrieve it from the ledger instead of the file (but keep the cache feature!). I saw you're already reading the params from the ledger in AddPublicParamsDependency, so I thought maybe a comparison could make sense as a sanity check for the config (because these things are hard to debug). But we can forget about this part for now, it's not so important.

So, to summarize, I find it risky to allow anyone to submit new public parameters. The honest endorsers would have no way to check if they must accept these params or not.

I don't mean that the invoke would allow the user to submit the parameters as argument of the invoke. That would indeed be a bad idea. It would have a few steps:

  1. There is offline agreement to update the parameters.
  2. Each endorser manually updates the file that the chaincode reads in here: https://github.com/hyperledger-labs/fabric-token-sdk/blob/2e556dd0f1dd48252e6874753e33e5be7947eae1/token/services/network/fabric/tcc/tcc.go#L143 to the new parameters. This is the essential part: human oversight, but not one-sided because it's not in use yet.
  3. Someone (doesn't matter who) invokes the 'init' or 'update' function.
  4. The invocation causes the file to be written to the ledger. Because a quorum of endorsers has the same read/writeset (because they each used their new file), the transaction succeeds and is committed.
  5. Hopefully the network is well organized and everybody has updated the params. In that case, all is well. Otherwise, endorsers who haven't updated their params will not be able to endorse new transactions. We could add a time-to-live to the in-memory cache so that they'll automatically recover within a reasonable timeframe by reading it from the ledger. Still, this is a very rare and specific case, and client applications should ideally already have some cleverness about switching endorsers...

But what if we say that the issuer can update the public params? Would this be fine? We can ask the token-sdk to check such a signature. Wdyt?

Do you mean the client application? When would it check it? If it only sees it after the new params have been committed, it could get a bit messy with differing points of view (what does it do if it rejects the new params?).

Issuer as authority makes sense, but maybe it's better to enforce it at the ledger level? Then we get back to either relying on the normal endorser quorum (which often includes the issuer organization as mandatory anyway) or key-based endorsement (I don't really like the added complexity - maybe someone might need it for specific cases but then they can just write the code for it ;) ).

adecaro commented 2 weeks ago

Let me make sure I understand. The proposal is: The administrator of each chaincode, update locally the public parameters and maybe a flag telling the chaincode that as soon as an Init command is received, it must be accepted and the flag reset. When this happens, anyone can submit the Init command maybe with the hash of the public params to activate and the switch happens. Something like this? Did I understand your proposal correctly?

arner commented 2 weeks ago

Yeah, except that I even don't think the flag and hash are necessary, they know to which params they update the file, right? Do we need an explicit vote or is the changing of the file your vote? However, now that you put it that way, we may not want the moment that the switch happens to be so free to choose for anyone...