Skip to content
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

Streamline token chaincode params initialization #743

Open
arner opened this issue Aug 27, 2024 · 6 comments
Open

Streamline token chaincode params initialization #743

arner opened this issue Aug 27, 2024 · 6 comments

Comments

@arner
Copy link
Contributor

arner commented Aug 27, 2024

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

  • Allow initializing token chaincode with a normal invoke instead of --init-required. It doesn't change the trust assumptions because init has the same endorsement policy as normal functions.
  • Let cc.Initialize get the params from the ledger, to enforce unity across endorsers and FSC nodes (makes it easier to reason about / debug). Log a warning if they don't match with the file (alternatively we could do that as part of AddPublicParamsDependency if the performance cost is low).
  • Allow the new init function to be called more than once, to update the params without deploying a new chaincode. It's easier to coordinate because endorsers can change the params file as part of normal operations instead of going through the install/endorse/commit/init ritual for new chaincode. There can be zero downtime if we let cc.Initialize use the ledger truth until we explicitly update it. Does it change the trust assumptions? From lifecycle endorsement policy (new chaincode version required), officially we go to the chaincode endorsement policy (writing a key) for updating params. But technically this is already the case: if a quorum signs a rwset with new params (bypassing the chaincode logic), that's the new committed reality for the network - also in the current setup.

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

@adecaro
Copy link
Contributor

adecaro commented Aug 27, 2024

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
Copy link
Contributor Author

arner commented Aug 27, 2024

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
Copy link
Contributor

adecaro commented Aug 27, 2024

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
Copy link
Contributor Author

arner commented Aug 27, 2024

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 (

if err := cc.Initialize(builtInParams); err != nil {
) 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:
    params := cc.ReadParamsFromFile()
    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
Copy link
Contributor

adecaro commented Aug 28, 2024

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
Copy link
Contributor Author

arner commented Aug 28, 2024

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants