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

[DX] module contracts/predictoor_contract.py & class PredictoorContract are confusing labels #812

Closed
2 tasks done
trentmc opened this issue Mar 18, 2024 · 0 comments · Fixed by #825
Closed
2 tasks done
Assignees
Labels
Type: Bug Something isn't working

Comments

@trentmc
Copy link
Member

trentmc commented Mar 18, 2024

Summary / TODO

  • rename contracts/predictoor_contract.py & PredictoorContract -> contracts/feed_contract.py & FeedContract
  • throughout pdr-backend, replace all use of predictoor_contract etc --> feed_contract (or feed when appropriate). Note that some modules already use this (eg predictoor bot)

Background

In ocean-contracts repo, we have ERC20Template3.sol. This is the datatoken template that implements Predictoor-style datafeeds.

We have deployed 20 of these contracts: 10 for 5min, 10 for 60min.

In this repo, the module contracts/predictoor_contract.py is a Python wrapper for ERC20Template3.sol. Its class is PredictoorContract.

Then throughout lots of this repo, we refer to those contracts as predictoor_contracts.

Here's the problem: they're feeds not predictoors.

Let's clarify. When we say "predictoors" in the context of Predictoor product, we are referring to the bots making the predictions, or to the people running those bots. We are not referring to the feeds. "Predictoor" is an inappropriate label.

One could say "but what about contracts used for the Predictoor system?"

  • It's a good Q. That label would work if we have just one or two types of contracts.
  • But it doesn't work if, for example, we need to distinguish between eg (1) ERC20Template3 data feed contracts and (2) special contracts that wrap behavior of a predictoor bot.
  • The latter would be more appropriately labeled "predictoor" (though ideally not there as well, since it might be too ambiguous!)

We need a better label!

Related Labels

Let's review other related labels.

First, here are related labels in pdr-backend repo (this repo):

  • contracts/erc721_factory.py, class Erc721Factory
  • contracts/data_nft.py, class DataNft
  • cli/arg_feed.py, class ArgFeed
  • subgraph/subgraph_feed.py, class SubgraphFeed

In ocean.py repo, models/ directory, related labels:

  • datatoken_base.py, class DatatokenBase. Children:
    • datatoken1.py, class Datatoken1
    • datatoken2.py, class Datatoken2
    • (note: these used to have more complicated labels, those were a huge pain, so we fixed it to ^)
  • erc721_token_factory_base.py, class ERC721TokenFactoryBase. Children:
    • data_nft_factory.py, class DataNFTFactoryContract
  • data_nft, class DataNFT

Goals

Goals for our labels:

  1. don't confuse with predictoor bots
  2. distinguish between erc20 / datatoken, vs erc721 / data_nft
  3. imply "feed"
  4. distinguish from arg_feed and subgraph_feed
  5. maybe imply "contract"
  6. not too verbose (keep # characters reasonable)
  7. grammatically acceptable / not confusing
  8. "datatoken3" or "ERC20Template3" are too constraining - we may upgrade to a datatoken4, and maybe in next 6 mos. If we did, they're likely an improved version of the current contracts, we wouldn't support bot hin pdr-backend

Candidate Labels --> Analysis

(Module name, class name) --> Analysis

  • (a) predictoor_contract.py, PredictoorContract (status quo) --> fail (1): confusion
  • (b) datatoken3.py, Datatoken3 --> fail (8): too constraining
  • (c) ERC20_template_3, ERC20Template3 --> fail (8): too constraining for upgrades
  • (d) feed.py, Feed --> fail (4): doesn't distinguish arg_feed / subgraph_feed
  • (e) prediction_feed_contract.py, PredictionFeedContract --> fail (6): too verbose
  • (f) contract_feed.py, ContractFeed -> fail (7): sounds like "contract killer", wrong order
  • (g) feed_contract.py, FeedContract -> no fail! Good!

Summary: the winner is: feed_contract.py, FeedContract

@trentmc trentmc added Type: Bug Something isn't working Priority: High labels Mar 18, 2024
@trizin trizin linked a pull request Mar 20, 2024 that will close this issue
trizin added a commit that referenced this issue Mar 20, 2024
* PredictoorContract -> FeedContract

* Rename to feed contract

* predictoor_contract -> feed_contract

* Black
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants