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

Add support for multiple contract ABIs #1491

Open
ra0x3 opened this issue Dec 4, 2023 · 7 comments
Open

Add support for multiple contract ABIs #1491

ra0x3 opened this issue Dec 4, 2023 · 7 comments

Comments

@ra0x3
Copy link
Contributor

ra0x3 commented Dec 4, 2023

@ra0x3
Copy link
Contributor Author

ra0x3 commented Dec 4, 2023

@segfault-magnet

  • Given how we use the JSON ABI I'm wondering if this is even possible?
  • We iterate over the JSON ABI to collect all types for a given JSON ABI
  • This would mean that collecting these types for multiple/separate JSON ABIs would lead to type collisions?
    • type A in contract1 might have type ID 4, and type B in contract2 might also have type ID 4
  • I know that we can take multiple JSON ABI paths and pass them to the SDK via multiple AbigenTargets, but I don't think it's possible to integrate all the other non-SDK indexer functionality - since it all works for types from a single contract
    • We'd have to get all types from all contract ABIs then effectively re-assign new TypeDeclarations to them, with new type IDs - which sounds fraught with danger? 🥲
      • Especially tricky with something like predicates where the original type ID (from the SDK) is important

@segfault-magnet
Copy link
Contributor

segfault-magnet commented Dec 4, 2023

  • Given how we use the JSON ABI I'm wondering if this is even possible?

I'm not familiar with how the JSON ABI is used by the indexer, except for the part where Abigen is called with the abi files.

  • We iterate over the JSON ABI to collect all types for a given JSON ABI
  • This would mean that collecting these types for multiple/separate JSON ABIs would lead to type collisions?
    • type A in contract1 might have type ID 4, and type B in contract2 might also have type ID 4
  • I know that we can take multiple JSON ABI paths and pass them to the SDK via multiple AbigenTargets, but I don't think it's possible to integrate all the other non-SDK indexer functionality - since it all works for types from a single contract

The SDK avoids type collisions by namespacing the generated types. Each Contract/Script or Predicate gets its own mod. Sometimes that is not enough since a contract can have multiple libraries with types that have the same name but different implementations. For that case forc needs to be called with a flag that will enable "call paths", this makes forc generate a JSON abi with the path of each custom type (i.e. "some::library::Error", and "some_other::library::Error" instead of just "Error" and "Error"). We use that information to generate mods for every type, just like they would have had over in Sway.

  • We'd have to get all types from all contract ABIs then effectively re-assign new TypeDeclarations to them, with new type IDs - which sounds fraught with danger? 🥲

I don't know the inner workings. The idea of assigning a TypeDeclaration to a type is unfamiliar. Over in the SDK, we generate a type from a TypeDeclaration and the TypeDeclaration is thrown away after that.

* Especially tricky with something like predicates where the original type ID (from the SDK) is important

Also not familiar with how predicates in the indexer would work.

@ra0x3
Copy link
Contributor Author

ra0x3 commented Dec 4, 2023

@segfault-magnet

not familiar

  • Understood 😆
  • I think this sort've answers my question - if we namespace everything we're doing using the mod that should get us pretty far in supporting this :) thanks agin

@ra0x3 ra0x3 added the P: Medium label Dec 4, 2023
@dmihal
Copy link

dmihal commented Dec 4, 2023

Do you have an example of what the manifest would look like with multiple ABIs?

Coming from The Graph, it's always a bit confusing that the manifest doesn't have a list of contracts. Would this change add that?

@ra0x3
Copy link
Contributor Author

ra0x3 commented Dec 4, 2023

@dmihal Definitely.

So with the merge of #886 the new manifest format will be

namespace: fuel_indexer_test
fuel_client: ~
schema: packages/fuel-indexer-tests/indexers/fuel-indexer-test/schema/fuel_indexer_test.graphql
start_block: ~
end_block: ~
contract:
  abi: packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract1-abi.json
  subscriptions:
    - fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23p4k
identifier: index1
module:
  wasm: target/wasm32-unknown-unknown/release/fuel_indexer_test.wasm
resumable: true
predicates:
  templates:
    - name: TestPredicate1
      abi: packages/fuel-indexer-tests/sway/test-predicate1/out/debug/test-predicate1-abi.json
      id: 0xcfd60aa414972babde16215e0cb5a2739628831405a7ae81a9fc1d2ebce87145
    - name: TestPredicate2
      id: 0xb16545fd38b82ab5178d79c71ad0ce54712cbdcee22f722b08db278e77d1bcbc
      abi: packages/fuel-indexer-tests/sway/test-predicate2/out/debug/test-predicate2-abi.json

And if we were to include multiple contract ABIs, it would simply be updated to:

namespace: fuel_indexer_test
fuel_client: ~
schema: packages/fuel-indexer-tests/indexers/fuel-indexer-test/schema/fuel_indexer_test.graphql
start_block: ~
end_block: ~
contract:
  abi: 
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract1-abi.json
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract2-abi.json
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract3-abi.json
  subscriptions:
    - fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23p4k
identifier: index1
module:
  wasm: target/wasm32-unknown-unknown/release/fuel_indexer_test.wasm
resumable: true
predicates:
  templates:
    - name: TestPredicate1
      abi: packages/fuel-indexer-tests/sway/test-predicate1/out/debug/test-predicate1-abi.json
      id: 0xcfd60aa414972babde16215e0cb5a2739628831405a7ae81a9fc1d2ebce87145
    - name: TestPredicate2
      id: 0xb16545fd38b82ab5178d79c71ad0ce54712cbdcee22f722b08db278e77d1bcbc
      abi: packages/fuel-indexer-tests/sway/test-predicate2/out/debug/test-predicate2-abi.json

@dmihal
Copy link

dmihal commented Dec 5, 2023

So is fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23p4k the contract that's being indexed? How would it look if there's multiple contracts being indexed, eich with it's own ABI?

Is there a reason we don't have a contracts section similar to the predicates section?

@ra0x3
Copy link
Contributor Author

ra0x3 commented Dec 5, 2023

@dmihal By "similar to the predicates section" I think you're meaning tucked in as a Map object

- key1: value1
  key2: value2
  key3: value3
  • The thing is, indexers can just be subscribed to contract addresses without actually compiling with an ABI
    • They can also be subscribed to contracts that are totally different from the contract they're being compiled with
  • So I think what you're talking about might be (?) :
contracts:
  - address: 0xcfd60aa414972babde16215e0cb5a2739628831405a7ae81a9fc1d2ebce87145
    abi: packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract3-abi.json
  - address: 0xcfd60aa414972babde16215e0cb5a2739628831405a7ae81a9fc1d2ebce87155
    abi: packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract2-abi.json
  • But without a subscriptions section (or something similar) you'd be required to only subscribe to contracts that the indexer is compiled with
    • E.g., the explorer backend isn't subscribed to any particular contract ID
  • So the idea with the approach I pasted... :
contract:
  abis: 
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract1-abi.json
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract2-abi.json
    - packages/fuel-indexer-tests/sway/test-contract1/out/debug/test-contract3-abi.json
  subscriptions:
    - fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23K4k
    - fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23Z4k
    - fuel1vrwlw55qkc8xgn5sljjlawhlkqjgstczqwcyakx8a80c42tpqc8qv23M4k
  • ...is to separately mention which contract addresses the indexer is subscribing to (whether they be related to this indexer or not), which could or could not be related to the contracts that are being compiled with the indexer
  • Open to feedback on this, as the PR for this manifest change is still open

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

No branches or pull requests

3 participants