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

Advanced Object Deletion #1220

Open
nguyer opened this issue Mar 15, 2023 · 7 comments
Open

Advanced Object Deletion #1220

nguyer opened this issue Mar 15, 2023 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@nguyer
Copy link
Contributor

nguyer commented Mar 15, 2023

Background

While doing some testing locally, I ended up stuck in a less than ideal state. Here's a summary of what I did:

  • Created a "gateway mode" stack connected to a public testnet
  • Deployed an ERC-721 token contract that I created with the OpenZeppelin contract wizard
  • Created a token pool for that contract
  • Attempted to mint a token with a URI
  • FireFly told me (based on the response from the token connector) that setting a URI is not supported with my contract
  • This was due to the fact that I did not specify the contract interface when creating the token pool
  • I went back and created the interface
  • I then went to create a new token pool with a new name, while specifying the interface ID
  • The FireFly transaction for creating the token pool becomes stuck "Pending", while the Token Create Pool Operation is successful
  • Upon inspecting the logs I found a 200 response from the token connector to create the pool, then FireFly core logs: [2023-03-15T16:53:11.943Z] DEBUG Token pool ID=79a6bb2f-11bd-45e6-99a7-f3045ec911ac Locator='address=0xfd403b063ecf959867973a4cf383c48b9a06e19e&schema=ERC721NoData&type=nonfungible' already confirmed ns=default pid=1 role=event-manager

This now leaves me stuck state. I have no way of deleting the token pool, but because I created it incorrectly, I also have no way of using it. My options at this point are to:

  • Deploy a new contract - not viable if this was a pre-existing contract that already has transactions that need to be index
  • Completely delete my stack and start over - not viable if I have other production data in my DB
    • Worse, I don't think this would even work if I was in a multi party system, because as soon as my FireFly node comes back online it would see the token pool broadcast in the chain as it re-indexes everything

In reality, this problem extends to more than just token pools, but this situation was exceptionally painful, so it got me thinking about the problem. The same situation could occur with contact interfaces, contract APIs, etc, though those things have versions so it may not be as big of a deal there.

What needs to change

  • We need a way to delete things which have not been broadcasted, which should be a simple curd operation.
  • We also need a way to delete a token pool which has been broadcasted
@nguyer nguyer added the enhancement New feature or request label Mar 15, 2023
@awrichar awrichar self-assigned this Apr 4, 2023
@awrichar
Copy link
Contributor

awrichar commented Apr 4, 2023

Full list of definition types that this would encompass:

  • datatype
  • contract interface
  • contract API
  • identity
  • token pool

And a summary of some specific enhancements that were discussed offline among maintainers:

  1. Split the concept of "creating a definition" from "publishing a definition" and introduce a new set of APIs for the latter (such as POST /tokens/pools/{id}/publish)
  2. Add a query param to all definition creation APIs (such as ?publish=true) to publish the definition while creating it (ie current behavior)
  3. Add a namespace-level config (such as multiparty.autoPublish=true) to publish all definitions when they are created (ie current behavior)
  4. Add delete APIs for all definition types (such as DELETE /tokens/pools/{id})
    a. Allow usage for unpublished definitions
    b. Allow usage for published definitions

Uniqueness of names will be a problem that needs addressing, in the case that each member may have a combination of published and unpublished definitions.

The verb for this action should also be carefully chosen. I've used the word "publish" here, but other options are "broadcast" or "announce".

@awrichar
Copy link
Contributor

awrichar commented Apr 4, 2023

Notes on possible verbs and any current usage:

  • "publish" - used to describe the act of pushing a data value or blob into IPFS
  • "broadcast" - used to describe all messages that are sent to the entire multiparty network
  • "announce"- used internally around some token pool definition logic (but not used widely or externally as of yet)
  • "share" - used in docs like "broadcast shared data", and used in the term "shared storage" (but currently not used as a standalone verb anywhere)

@awrichar
Copy link
Contributor

awrichar commented Apr 5, 2023

Looking to (4) - the actual "delete" portion - my first instinct would be to allow deleting anything, regardless of whether it's been published or not. That is, I can choose to delete a "published/shared" token pool or API from my node, and all record of it will be removed. This puts me voluntarily out-of-sync with the rest of the multiparty network, but that is my choice as a node operator. This tracks with the decisions that were made around deletion of data.

This also implies that after deleting a "shared" item, I could reuse that name for my own local item. However, if I then tried to publish mine, it would be rejected by all other nodes on the network. That is, unless any of those nodes had also deleted the original shared item, in which case they would accept my new one.

Obviously there are loads of ways to mess up your network, but I'm trying to decide if we should actually prevent them or if we should leave that to the user.

@awrichar
Copy link
Contributor

awrichar commented Apr 7, 2023

Alternative proposal that gives less ways for nodes to be out of sync:

  • All definition types get an additional column networkName, which defaults to the same as name, but can also be specified when you publish.
  • networkName and name must each be locally unique.
  • Whenever you receive a definition whose networkName is unique but clashes with the name of an existing item, generate a unique name by adding an index to the networkName.
  • Do not allow deletion of published items by any member.

Notably this drops (4b) from scope, but that could always be revisited later.

@awrichar
Copy link
Contributor

Coming back to the naming question after fleshing out #1261 - I went for "publish" and it feels fairly correct. The API spelling lines up with how the word is used around data/blobs, and the functionality is similar enough (IMO). For clarity, this is what "publish" would now mean in both contexts:

  • "publish a data item" - upload this item to IPFS and record the public ref for me
  • "publish a token pool" - serialize the definition of this token pool, upload it as as message batch to IPFS, and broadcast its existence to all other members of the multiparty network

Obviously they're a bit different, but in both cases we're taking something local and making it available via IPFS.

Side note: As part of this change, I'm also removing usage of the word "announce" and switching all token pool verbage to use "publish" instead.

@peterbroadhurst
Copy link
Contributor

Did some 🤔 on the naming independently on prompting from the PR comment on #1261, and came to a pretty identical conclusion.

@awrichar
Copy link
Contributor

awrichar commented May 25, 2023

This work is now completed or in flight for token pools, contract interfaces, and contract APIs:

Of the original items proposed at the start, this implements 1, 2, 4a for these types. The others (3, 4b) are deferred for now. Datatypes and identities also need a look, as they may come with special considerations that influence multiparty messaging.

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

No branches or pull requests

3 participants