ocean.py handling of datatoken templates is error-prone and hard to extend

[trentmc]

Background / motivation

This issue was partly spurred by discussion in PR 1304, but also something that's been brewing for a while in my head:)

Here are the problems:

• The current class setup of Datatoken and DatatokenEnterprise is messy. "Datatoken" class both defines the (a) baseline interface for all Ocean datatokens in Python, and (b) implements template 1 for it. Then "DatatokenEnterprise" class reuses some of it, but to understand you have to review both libraries.
• The labels are poor. "Datatoken" class labels both (a) interface of all datatokens, and (b) implementation for template 1. And "DatatokenEnterprise" labels template 2, and there's little correlation with "enterprise"; and having "enterprise" in its label is counter-productive.

Here's another way to illustrate the problem: imagine if we wanted to add another template. It would be super-messy, because of all the weird relations above. (And, there's a very high chance we do want to add templates, probably in the next 6 months. So this is a real challenge.)

This issue is not urgent to solve, though it is important. And it will become urgent if we want to add another template.

Towards a solution

There's a better way:

• have an abstract class "Datatoken" or "AbstractDatatoken" that defines the interface
• have classes "Datatoken1" and "Datatoken2" for templates 1 and 2 respectively. Each of these inherits (i) the abstract class for interface definition, and (ii) appropriate mixins with the implementations as needed.
• then implement the mixins too. There will probably be a few. It'll become obvious as you build it.

TODOs

• Build "towards a solution" as a prototype, get feedback, iterate until good
• Update READMEs and documentation

Appendix / Notes

I recognize that the smart contracts themselves are named similarly confusingly: ERC20Template.sol and "ERC20TemplateEnterprise.sol". That's harder to change given that these are deployed immutably. I do recommend that future templates get named "ERC20Template3.sol", "ERC20Template4.sol", etc.

[trentmc]

I just realized that the merged commit solves the first bullet issue (current setup is messy), but only partially the second bullet (naming).

Here's the second bullet in full :

• The labels are poor. "Datatoken" class labels both (a) interface of all datatokens, and (b) implementation for template 1. And "DatatokenEnterprise" labels template 2, and there's little correlation with "enterprise"; and having "enterprise" in its label is counter-productive.

The label "Datatoken" and "Datatoken" enterprise are still problematic.

Here's another way to illustrate the problem: imagine if we wanted to add another template. Its labeling compared to the others would be super confusing.

Proposed solution: have classes "Datatoken1" and "Datatoken2" for templates 1 and 2 respectively.

I'm an re-opening this in order to solve this.

[calina-c]

@trentmc I do not think this is the most fortunate naming. It can be easily typoed and badly autocompleted. Do you have any other suggestions? I would add something like DatatokenSimple vs. DatatokenEnterprise. Not necessarily these ones, but clearer labels instead of numbers.

[trentmc]

I'm open to exploring other names.

First, let's refresh on differences between template 1 and 2:

• Big difference: template 1 follows the mental model of "get datatokens" then "pass them around" then "consume". Template 2 merges "get datatokens" (via faucet or exchange) with "consume", and has no passing around.
• Specific: Faucet / Dispensing:
  • Template 1: Anyone can call dispense_and_order() which takes 2 txs. Anyone can call dispense() to request tokens.
  • Template 2: Anyone can call dispense_and_order()which takes 1 tx. Not anyone can call dispense() by default; to allow anyone, the publisher must first call: ocean.dispenser.setAllowedSwapper(datatoken_address, ZERO_ADDRESS, {"from" : publisher_wallet}), where ZERO_ADDRESS is 0x00..00.
• Specific: Exchange / Selling:
  • Template 1: Anyone can call buy_DT_and_order() which takes 2 txs
  • Template 2: Anyone can call buy_DT_and_order() which takes 1 tx

Also, let's consider possible new templates (that we could realistically see within the next 6-12 mos):

• Template 3: like template 2, plus: anyone can call dispense() by default. (Via allowed_swapper = ZERO_ADDRESS by default)
• Template 4: like template 1, plus: allows non-custody of url, via: Provider doesn't ever see plaintext url via Lit Procol threshold cryptography
• Template 5: like template 3, plus: allows non-custody of url, via ...

Candidates for template 1:

1. Datatoken (status quo)
2. Datatoken1
3. (a) DatatokenTemplate1 (b) Template1Datatoken
4. (a) DatatokenSimple (b) SimpleDatatoken
5. (a) DatatokenBasic (b) BasicDatatoken
6. (a) DatatokenOriginal (b) OriginalDatatoken
7. (a) DatatokenGenesis (b) GenesisDatatoken

Candidates for template 2:

1. DatatokenEnterprise (status quo)
2. Datatoken2
3. (a) DatatokenTemplate2 (b) Template2Datatoken
4. (a) DatatokenNotoken (b) NotokenDatatoken
5. (a) DatatokenNoCustody (b) NoCustodyDatatoken
6. (a) DatatokenCustodyLight (b) CustodyLightDatatoken
7. (a) DatatokenNoncustodial (b) NoncustodialDatatoken
8. (a) DatatokenMergedAcquireAndOrder (b) MergedAcquireAndOrderDatatoken
9. (a) DatatokenCombinedTx (b) CombinedTxDatatoken
10. (a) DatatokenMergedTx (b) MergedTxDatatoken

And then we'd also need to come up with potential names for:

• template 3
• template 4
• template 5

Some analyses of names:

• "Datatoken" on its own is a terrible label because it overlaps with the base concept of datatoken
• "DatatokenEnterprise" is a terrible label because there's nothing specifically "enterprise" about it.
• Variants of names for template 2 that have "custody" or "merged tx" are not great because each name only points to some of the characteristics. And if we picked one of those, what would we pick for names that riff on template 2 (ie template 3, or template 5)

Here's my recommendation:

• use Datatoken1, Datatoken2, ..., Datatoken5
• and have a table in ocean.py README, and Ocean docs, that summarizes their uses / differences, like shown below

Template #	Class Label	Allows dispense by default?	Allows non-custody of datatokens?	Combines txs?	Allows non-custody of url?
1	Datatoken1	Y	N	N	N
2	Datatoken2	N	Y	Y	N
3	Datatoken3	Y	Y	Y	N
4	Datatoken4	Y	N	N	Y
5	Datatoken5	Y	Y	Y	Y