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

Support multiple network definitions inside a Package #351

Closed
abourget opened this issue Dec 1, 2023 · 7 comments
Closed

Support multiple network definitions inside a Package #351

abourget opened this issue Dec 1, 2023 · 7 comments
Labels

Comments

@abourget
Copy link
Contributor

abourget commented Dec 1, 2023

Rationale

It is very cumbersome right now to specify the configuration, initial blocks and parameterization for each networks. They need to be in separate files, and we get into a combinatorial problem when we also try to do sink configurations, for each network.

With the current model, we are also forced to ship a new spkg for each network configuration, making substreams.dev's interface a bit more ugly. Ideally, we would select an spkg, which is known to work for multiple networks, and through a dropdown, for a given version, we could select which network to use dynamically.

This puts the onus on the client sending the request to pick up the configs for a given network (taken from network or as a command-line argument to override it), and apply them to the right modules. The next-level overrides will come from the command-line p flags.

Proposition

Support this syntax in the manifest:

imports:
  erc20: spkg.io/streamingfast/erc20.spkg
  src: spkg.io/streamingfast/uniswap-v3-v1.2.3.spkg  # Now upon importing, we also namespace and import the `networks` definitions.

modules:
- name: my_mapper
  initialBlock: 123123
  inputs:
  - params: string
    value: "alskdjfalskdj"

network: mainnet

networks:
  mainnet:
    initialBlocks:
      # we've got an implicit definition of
      #  src:map_pair_created: 76098098
      # from the import of `src` up there ^^ And it needs to be defined for ALL networks we're going to query here.
      module1: 123123
    params:
      module1: "address=0x123123123123"
  goerli:
    initialBlocks:
      module1: 234234
    params:
      module1: "address=9x234234234"

The deriveFrom directive would allow one to either OVERWRITE or APPEND a network configuration. Or to replace the functionality of deriveFrom, one could now use substreams (re?)pack --network polygon ./substreams.yaml which would write a new .spkg, preconfigured (with network = polygon, and all parameters and initial block in place in their Module definition), with the filename appended with -polygon.spkg (before the version? after the version?). This ought NOT TO CHANGE the package.name to be able to deduplicate once on substreams.dev ...

  • Maybe deriveFrom just goes away, as it serves no other purpose.

The run and gui commands would now support --network and apply the network configs to the Modules before sending the request over. It would also do some checks to make sure the overrides of params and initialBlocks are consistent between networks (no keys are missing for instance).

The imports manifest declaration needs to also import the networks definitions, and remap the module_name to imported_name:module_name .. so it cascades when someone selects a network from a child spkg (one that imports multiple spkgs for instance).

  • The network application, will then apply the changes for all of its parents.

Validation for substreams.yaml's networks:

  • build a set of all keys, for each network
  • the set of keys need to be equal for each network, so no missing key.
  • This means that: if you define a module that needs override for a given network, it needs to be defined for all networks.
  • This also means we will not validate or force the overriding of the initialBlock or original params value for a module, unless it uses this networks override mechanism.

In the above example, if goerli was not defined within the src import, the algorithm above would halt with the error: missing initialBlock for network "goerli", on module "src:map_pair_created".


  • That is still annoying because we don't want to see published all sorts of spkg for individual networks. substreams.dev can cross-check my comparing module count, and module hash for each network in equality, + version and package.name.
  • Selected a network is a deploy-time concern, not a package-time concern. If we do that, we still encourage the packaging of an spkg per network, which complexifies and makes substreams.dev ugly and unintuitive.

Q: Rename network to defaultNetwork or selectedNetwork? Keep it as is? preconfiguredNetwork

@abourget
Copy link
Contributor Author

abourget commented Dec 1, 2023

One could think that the init codegen could write a parameterized module, and put the contract address in the networks section, along with the initialBlock, for the selected network. This way, the scaffolded code would be ready to accomodate other networks.

@YaroShkvorets
Copy link
Contributor

This would be great. substreams init could keep asking user for networks in a loop verifying that ABIs match and querying initialBlock, and add entries to the networks section.

@abourget
Copy link
Contributor Author

abourget commented Dec 1, 2023

Ok, we refined and updated the thing, @YaroShkvorets please re-read.

@sduchesneau
Copy link
Contributor

sduchesneau commented Dec 15, 2023

After more discussions:

  1. we can't validate that a) all networks have the same modules defined in them, nor b) that all the existing modules have the same values in all networks.

This is because:
a) and b) I may create a substreams that supports only a subset of the networks that an imported .spkg does. I wouldn't have to define values the other networks that I don't want to serve.
b) an .spkg that I import may have network-defined values for some modules, but my own modules do not depend on them -- instead, they depend only on modules that are not affected by the values in networks.

  1. we have to keep ALL the networks overrides of the packages that we import, so that someone could import our spkg from another package and fill in the blanks to make it work...

  2. the default values of a module for initialBlock and Params are redundant if they are overriden in Networks (with a default network set -- which will be mandatory).

This leads to the following decisions.

  • 1. When running a substreams, we will check "global" consistency only for the modules that are required to run the requested output_module (looking at the graph). When packing a substreams, we will do the same for all the modules and networks that are declared locally, and the modules on which they depend.
  • 2. The substreams client should fail when packing the default values of a module are different than the ones that are overriding them.
  • 3. When setting per-network values, the ones that correspond to the default network should be written over the ones directly written in the module when these ones don't exist or when overriden from another module.

@sduchesneau
Copy link
Contributor

Eventually, we could remove the client-side burden of computing all this and send the packed spkg in the request, along with the parameters like 'network' and 'params'. Maybe...

@sduchesneau
Copy link
Contributor

#358 reviews welcome!

@ghardin1314
Copy link

This is great and needed for a long time for us! I think one last touch is to update the schema.json file to reflect these changes. I can make a PR to update but it may be a couple days before I get back around to it

https://github.com/streamingfast/substreams/blob/develop/schemas/manifest-schema.json

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

No branches or pull requests

4 participants