Fuel merge part 6 - Codegen fuel event type - #155

DZakh · 2024-08-29T14:47:56Z

Also, it cleans up our usage on Ethers. No need to create an Ethers.Interface on every getLogs call
Untangled system_config::Event type from the Evm abi event

Types in the handler work 🔥

Almost 😅

It still doesn't compile, because the rescript-schema is not generated. Also, there needs to be an adjustment for the TestHelpers module. But after it's done, I'll start working on making the engine work 🚀

DZakh · 2024-08-29T14:53:20Z

@@ -0,0 +1 @@
+


Kept the file, so it's overwritten on the codegen (locally)

Hmmm, does that mean we should always have empty files for things we remove? I would suggest just remove it. The cli handles cleaning stale generated files if the version changes.

DZakh · 2024-08-29T14:54:09Z

 type interfaceAndAbi = {
-  interface: Ethers.Interface.t,
  abi: Ethers.abi,
+  sighashes: array<string>,
 }


This module doesn't look very useful anymore, probably we should refactor it in the future.

DZakh · 2024-08-29T14:54:23Z

-let getAbiMapping = (self: t) => {
-  self.contractAddressMapping.nameByAddress
-  ->Js.Dict.entries
-  ->Belt.Array.keepMap(((addr, name)) => {
-    self.contractNameInterfaceMapping->Js.Dict.get(name)->Belt.Option.map(v => (addr, v.abi))
-  })
-  ->Js.Dict.fromArray
-}


DZakh · 2024-08-29T14:55:25Z

-    let {name, abi} = contract
-    let interface = Ethers.Interface.make(~abi)


With the change, we don't need to create the Ethers.Interface anymore. It was done at least on every getLogs request. Probably not a bottleneck, but definitely good to avoid it.

DZakh · 2024-08-29T14:56:24Z

-let getLogSelection = (self: t): result<array<LogSelection.t>, exn> => {
-  try {
-    self.contractAddressMapping.addressesByName
-    ->Js.Dict.keys
-    ->Belt.Array.map(contractName => {
-      let interfaceOpt = self->getInterfaceByName(~contractName)
-      switch interfaceOpt {
-      | None => UndefinedInterface(contractName)->raise
-      | Some({interface}) => {
-          let topic0 = []
-          //Add the topic hash from each event on the interface
-          interface->Ethers.Interface.forEachEvent((eventFragment, _i) => {
-            topic0->Js.Array2.push(eventFragment.topicHash)->ignore
-          })
-
-          let topicSelection = LogSelection.makeTopicSelection(~topic0)->Utils.unwrapResultExn
-
-          let addresses = []
-          //Add the addresses for each contract
-          self.contractAddressMapping
-          ->ContractAddressingMap.getAddressesFromContractName(~contractName)
-          ->Belt.Array.forEach(address => addresses->Js.Array2.push(address)->ignore)
-
-          LogSelection.make(~addresses, ~topicSelections=[topicSelection])
-        }
-      }
-    })
-    ->Ok
-  } catch {
-  | exn => exn->Error
-  }
-}


Sorry, it'll most likely conflict with your changes. But I hope the new version will be more convenient to work with (moved to HyperSync worker)

DZakh · 2024-08-29T14:56:45Z


-// TODO: Remove in v3
 @genType.import(("./OpaqueTypes.ts", "Address"))
+@deprecated("Use Address.t instead. The type will be removed in v3")


Remembered about the attribute :)

JonoPrest · 2024-08-29T19:21:34Z

Thanks @DZakh, I'll do a review in the morning.

JonoPrest · 2024-08-30T09:18:00Z

+        // TODO: Clean up fuel_abi to include only relevant events
+        Ok((events, fuel_abi))


I guess it's probably more complicated with a fuel abi to "treeshake" since there are lots types referencing each other.

I think the main reason I did it with evm code was so that there weren't some unknown discrepancies between defining values inline vs with a json ABI.

It's possible, since there are a lot of things in the ABI we don't use

JonoPrest · 2024-08-30T09:23:37Z

-#[derive(Debug, Clone, PartialEq)]
-struct NormalizedEthAbiEvent(EthAbiEvent);


I see you've removed these type semantics. Just wanted to make you aware of the new type idiom in rust if you're not familiar: https://doc.rust-lang.org/rust-by-example/generics/new_types.html

It's useful in the sense that it shows when I use this type elsewhere that it's been parsed

JonoPrest · 2024-08-30T09:29:13Z

+                let signatures = abi.get_event_signatures();
+
+                format!(
+                    "let abi = Ethers.makeAbi((%raw(`{}`): Js.Json.t))\nlet eventSignatures = [{}]",


I don't really mind too much but sometimes these multiline template strings read easier with a rust raw string. Kind of like template strings in js. https://rahul-thakoor.github.io/rust-raw-string-literals/

I'll change in the next PR

JonoPrest · 2024-08-30T09:39:27Z

+        let topicSelection = LogSelection.makeTopicSelection(~topic0=contract.sighashes)->Utils.unwrapResultExn
+


Can we rather not unwrap here and then we don't need a try catch block below?

Then we'll need to convert array<result<_>> to result<array<>>, so I prefer throwing exception. The array is not big here, but still I don't want to add unnecessary performance overhead.

JonoPrest

Hey @DZakh, looks good so far! Nothing major just a couple of comments.

JonoPrest

Awesome, looking good.

DZakh force-pushed the dz/fuel-merge-5 branch from 3d385e7 to e2a841b Compare August 29, 2024 14:51

DZakh requested a review from JonoPrest August 29, 2024 14:51

DZakh commented Aug 29, 2024

View reviewed changes

JonoPrest reviewed Aug 30, 2024

View reviewed changes

DZakh added 8 commits August 31, 2024 11:43

Connect fuel abi to the contract

778b247

Clean up raw abi codegen

2a617da

Clean up Ethers module

078093b

Remove Ethers.Interface usage and recreation on every logs request

3f09d78

Codegen abis from the rust side

2e102c2

Use Fuel abi to codegen types

432c836

Fix tests

92312d4

Return RegisterHandlers changes

23f6190

DZakh force-pushed the dz/fuel-merge-5 branch from 1e597a0 to 23f6190 Compare August 31, 2024 07:52

DZakh added 2 commits September 2, 2024 11:13

Merge branch 'main' into dz/fuel-merge-5

c4288ba

Remove Abis.res

9bce5fa

DZakh requested a review from JonoPrest September 2, 2024 07:19

Merge branch 'main' into dz/fuel-merge-5

3de33a0

DZakh enabled auto-merge (squash) September 2, 2024 08:13

DZakh commented Sep 2, 2024

View reviewed changes

Comment thread codegenerator/cli/templates/static/codegen/src/eventFetching/chainWorkers/HyperSyncWorker.res Outdated

DZakh commented Sep 2, 2024

View reviewed changes

Comment thread codegenerator/cli/templates/static/codegen/src/eventFetching/chainWorkers/HyperSyncWorker.res Outdated

Apply suggestions from code review

ef5f307

JonoPrest approved these changes Sep 2, 2024

View reviewed changes

DZakh merged commit 3fe26a8 into main Sep 2, 2024

DZakh deleted the dz/fuel-merge-5 branch September 2, 2024 08:24

		let {name, abi} = contract
		let interface = Ethers.Interface.make(~abi)

		// TODO: Clean up fuel_abi to include only relevant events
		Ok((events, fuel_abi))

		#[derive(Debug, Clone, PartialEq)]
		struct NormalizedEthAbiEvent(EthAbiEvent);

		let topicSelection = LogSelection.makeTopicSelection(~topic0=contract.sighashes)->Utils.unwrapResultExn

Uh oh!

Conversation

DZakh commented Aug 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonoPrest commented Aug 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonoPrest Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonoPrest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JonoPrest left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JonoPrest Aug 30, 2024 •

edited

Loading