feat: index MerkleDistributor `Claimed` events #110

dohaki · 2022-11-01T14:44:47Z

This adds the required entities and consumers for processing Claimed events of the MerkleDistributor contract.

package.json

nicholaspai · 2022-11-01T18:18:36Z

src/modules/web3/services/EventsQuerier.ts

+   * getSamplesBetween(1, 9, 3) //returns [[1, 3], [4, 7], [8, 9]]
+   * ```
+   */
+  private getSamplesBetween = (min: number, max: number, size: number) => {


You might want to look into using well-tested utility libs like lodash for these libs

nicholaspai

Approving so as not to block but want to see @amateima review on the higher level architecture changes that I'm not as familiar with.

The Claimed event is the correct event to query. Are there other event props that would be useful to add before we deploy the contract?

dohaki · 2022-11-02T18:22:20Z

The Claimed event is the correct event to query. Are there other event props that would be useful to add before we deploy the contract?

AFAICT it has everything that is needed.

james-a-morris

Left one nit and two questions about the database queries. Looks great 😎

src/modules/scraper/model/MerkleDistributorProcessedBlock.entity.ts

src/modules/scraper/model/claim.entity.ts

james-a-morris · 2022-11-03T16:53:15Z

src/modules/configuration/index.ts

      chainId: Number(process.env.MERKLE_DISTRIBUTOR_CHAIN_ID || "5"),
      referralsStartWindowIndex: Number(process.env.REFERRALS_START_WINDOW_INDEX || "1"),
-      startBlockNumber: 7866869,
+      startBlockNumber: 7884371,


Should this variable be stored in the ENV?

amateima · 2022-11-06T19:52:03Z

.env.sample

+MERKLE_DISTRIBUTOR_CHAIN_ID=
+MERKLE_DISTRIBUTOR_ADDRESS=


I think we should take into consideration the possibility of having multiple MerkleDistributor contracts in the future. Maybe we should replace these env vars with a single variable: MERKLE_DISTRIBUTOR_CONTRACTS=[{ address, chainId, ... }]

Yea, I also initially thought about that. But I couldn't think of a use case for supporting multiple MerkleDistributor contracts 🤔 My thought process was:

The referral rewards are based on a single aggregation of indexed deposits. If we would have multiple types of rewards then I can see a use case. For example, referral rewards of type A are distributed via a different MD than rewards of type B.

Therefore given a single type of referral reward, one configurable MD made more sense to me. It is still configurable, so if we want to change our stage env to use mainnet deployment, we could still do that.

amateima · 2022-11-06T19:52:28Z

.env.sample

+# MerkleDistributor overrides
+MERKLE_DISTRIBUTOR_CHAIN_ID=
+MERKLE_DISTRIBUTOR_ADDRESS=
+REFERRALS_START_WINDOW_INDEX=


Not sure what REFERRALS_START_WINDOW_INDEX is used for

I primarily added this to make the offset to determine which claims to consider for the rewards reset configurable. E.g. only consider Claimed events starting from window index 1. Maybe the intent gets clearer here.

But now that I think about it, we would need to re-run a migration if that env var changes. So maybe this isn't a valid use case anymore.

I also thought this could be good for testing purposes where we could shift windows for different rounds of Airdrop + Referrals claims in single MD deployment. E.g.

1st testing round Airdrop window index = 0, referrals start index = 1

2nd testing round referrals start index = 2

3rd round official QA airdrop = 3, referrals = 4

amateima · 2022-11-06T19:53:47Z

migrations/1667312452969-Claim.ts

+    await queryRunner.query(
+      `ALTER TABLE "merkle_distributor_recipient" DROP CONSTRAINT "UK_merkle_distributor_recipient_merkleDistributorWindowId_addre"`,
+    );


why is this constraint being dropped?

I also wondered about this. But it seems that Postgres has a max identifier length of 63 bytes and the originally defined key is of length 65. So it truncates it when created but doesn't when dynamically created by TypeORM. I will change the key to be shorter

Should not occur in the future. See 3615420

amateima · 2022-11-06T19:55:42Z

migrations/1667312452969-Claim.ts

+    await queryRunner.query(
+      `CREATE TABLE "claim" ("id" SERIAL NOT NULL, "caller" character varying NOT NULL, "accountIndex" integer NOT NULL, "windowIndex" integer NOT NULL, "account" character varying NOT NULL, "rewardToken" character varying NOT NULL, "blockNumber" integer NOT NULL, "claimedAt" TIMESTAMP NOT NULL, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), CONSTRAINT "UK_claim_windowIndex_accountIndex" UNIQUE ("windowIndex", "accountIndex"), CONSTRAINT "PK_466b305cc2e591047fa1ce58f81" PRIMARY KEY ("id"))`,
+    );


nit: can we format the query to use multiple lines? (see other migration files). It makes the queries easier to read

Ok, yea can do that. Is there a way to automate this somehow in TypeORM? 🤔

Manually changed the format for now. See 3615420

We can do that manually only :(

amateima · 2022-11-06T20:00:46Z

migrations/1667312452969-Claim.ts

+    await queryRunner.query(
+      `ALTER TABLE "merkle_distributor_recipient" ADD CONSTRAINT "UK_merkle_distributor_recipient_merkleDistributorWindowId_address" UNIQUE ("merkleDistributorWindowId", "address")`,
+    );


I think this constraint is already added in a previous migration here

Related to #110 (comment)

amateima · 2022-11-06T23:35:02Z

src/modules/scraper/service.ts

+    const chainId = this.appConfig.values.web3.merkleDistributor.chainId;
+    const configStartBlockNumber = this.appConfig.values.web3.merkleDistributor.startBlockNumber;
+    const provider = this.providers.getProvider(chainId);
+    const latestBlock = await provider.getBlock("latest");


I think we can avoid fetching the latest blocks again, since they were already fetched in the publishBlocks() function. We can call the getLatestBlocks() at the beginning of the run() function and pass the result to this.publishBlocks() and this.publishMerkleDistributorBlocks()

amateima · 2022-11-06T23:42:38Z

src/modules/scraper/service.ts

          await this.publishBlocks();
        }
+        if (this.appConfig.values.enableMerkleDistributorEventsProcessing) {
+          await this.publishMerkleDistributorBlocks();


This is just an observation, but it's weird that this.publishBlocks() and this.publishMerkleDistributorBlocks() looks different. Basically they do the same thing (only the repositories and the configured start block are different), but one is a 10 line function and the other one looks more complicated than it actually is. I don't have a problem leaving the code as it is, but I see an opportunity here in organising the code a little bit better. We can chat in private about it. It's clear that the original code wasn't as flexible and modular as it had to be

I refactored it a bit to reuse the same functions. 0cb04e3

amateima · 2022-11-06T23:43:48Z

src/modules/web3/services/EthProvidersService.ts

-    const supportedChainIds = Object.keys(this.appConfig.values.web3.providers);
-
-    for (const chainId of supportedChainIds) {
-      if (this.appConfig.values.web3.providers[chainId]) {
-        const provider = new ethers.providers.JsonRpcProvider(this.appConfig.values.web3.providers[chainId]);
-        this.providers[chainId] = provider;
-      }
-    }
-
-    for (const chainId of Object.keys(this.getProviders())) {
-      const spokePool = SpokePool__factory.connect(
-        appConfig.values.web3.spokePoolContracts[parseInt(chainId)].address,
-        this.getProvider(parseInt(chainId)),
-      );
-      this.spokePoolEventQueriers[chainId] = new SpokePoolEventsQuerier(spokePool);
-    }
+    this.setProviders();
+    this.setSpokePoolEventQueriers();
+    this.setMerkleDistributorEventQuerier();


Nice touch 👍

amateima · 2022-11-06T23:48:31Z

src/modules/web3/services/EthProvidersService.ts

+    for (const chainIdStr of Object.keys(this.getProviders())) {
+      const chainId = parseInt(chainIdStr);
+      const spokePoolAddress = this.appConfig.values.web3.spokePoolContracts[chainId]?.address;
+      if (spokePoolAddress) {
+        const spokePool = SpokePool__factory.connect(
+          this.appConfig.values.web3.spokePoolContracts[chainId].address,
+          this.getProvider(chainId),
+        );
+        this.spokePoolEventQueriers[chainId] = new SpokePoolEventsQuerier(spokePool);
+      }
+    }


A similar approach should be used for instantiating the MerkleDistributorEventQuerier objects. As a side note, setSpokePoolEventQueriers is not bullet proof either because it allows having defined only a single SpokePool contract per chain. Soon we'll have deployed a second SpokePool contract on mainnet. This is why an array for the MerkleDistributor contracts is better than the object that I used to define the config for the SpokePool contracts

amateima · 2022-11-06T23:53:26Z

src/modules/web3/services/EventsQuerier.ts

+    do {
+      const blockRangeSizeAtStart = this.blockRangeSize;
+      try {
+        retryWithLowerBatchSize = false;
+        events = [];
+
+        if (this.blockRangeSize) {
+          const intervals = this.getSamplesBetween(from, to, this.blockRangeSize);
+          // query events only for the first interval to make sure block range is fine
+          const [intervalStart, intervalEnd] = intervals[0];
+          const newEvents = await this.contract.queryFilter(filters, intervalStart, intervalEnd);
+          events.push(...newEvents);
+
+          // query the rest of block intervals in parallel in order to get the events
+          const newEventsList = await Promise.all(
+            intervals
+              .slice(1)
+              .map(([intervalStart, intervalEnd]) => this.contract.queryFilter(filters, intervalStart, intervalEnd)),
+          );
+          events.push(...newEventsList.flat());
+        } else {
+          const newEvents = await this.contract.queryFilter(filters, from, to);
+          events.push(...newEvents);
+        }
+      } catch (error) {
+        if (
+          (error as Web3Error).error?.code === Web3ErrorCode.BLOCK_RANGE_TOO_LARGE ||
+          (error as Web3Error).error?.code === Web3ErrorCode.EXCEEDED_MAXIMUM_BLOCK_RANGE ||
+          (error as Web3Error).error?.code === Web3ErrorCode.LOG_RESPONSE_SIZE_EXCEEDED ||
+          (error as Web3Error).error?.code === Web3ErrorCode.LOG_RESPONSE_SIZE_EXCEEDED_2
+        ) {
+          // make sure the block range size wasn't modified by a parallel function call
+          if (this.blockRangeSize === blockRangeSizeAtStart) {
+            const newBlockRangeSize = this.blockRangeSize ? this.blockRangeSize / 2 : DEFAULT_BLOCK_RANGE;
+            this.logger.warn(`lowering block range size from ${this.blockRangeSize} to ${newBlockRangeSize}`);
+            this.blockRangeSize = newBlockRangeSize;
+          }


From what I see this logic has not been modified, only moved, Right? I just want to double check because I don't see any difference and this is a crucial part used to query the events

Yes, exactly. This is only copied and not modified.

feat: index MerkleDistributor Claimed events

6045fad

dohaki requested review from amateima, james-a-morris and nicholaspai November 1, 2022 14:44

nicholaspai reviewed Nov 1, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

nicholaspai reviewed Nov 1, 2022

View reviewed changes

nicholaspai approved these changes Nov 1, 2022

View reviewed changes

dohaki mentioned this pull request Nov 2, 2022

feat: referral view updates (stage) #111

Merged

james-a-morris reviewed Nov 3, 2022

View reviewed changes

src/modules/scraper/model/MerkleDistributorProcessedBlock.entity.ts Show resolved Hide resolved

src/modules/scraper/model/claim.entity.ts Show resolved Hide resolved

src/modules/scraper/model/claim.entity.ts Show resolved Hide resolved

dohaki requested review from james-a-morris and nicholaspai November 3, 2022 14:20

chore: bump @across-protocol/contracts-v2

89645f1

james-a-morris reviewed Nov 3, 2022

View reviewed changes

dohaki added 2 commits November 4, 2022 11:39

fix: merkle distributor block events consumer

07b2008

refactor: make MerkleDistributor index start block configurable

393163c

dohaki requested a review from james-a-morris November 4, 2022 10:53

amateima requested changes Nov 6, 2022

View reviewed changes

dohaki added 3 commits November 7, 2022 16:44

refactor: split up migrations and adjust claim entity

3615420

refactor: add guards and rename

2c547f6

refactor: reuse logic in scraper service for block events

0cb04e3

amateima approved these changes Nov 7, 2022

View reviewed changes

dohaki merged commit 1ce720f into stage Nov 7, 2022

amateima deleted the index-claims branch May 4, 2023 18:15

		MERKLE_DISTRIBUTOR_CHAIN_ID=
		MERKLE_DISTRIBUTOR_ADDRESS=

feat: index MerkleDistributor Claimed events #110

feat: index MerkleDistributor Claimed events #110

Uh oh!

Conversation

dohaki commented Nov 1, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicholaspai left a comment

Choose a reason for hiding this comment

Uh oh!

dohaki commented Nov 2, 2022

Uh oh!

james-a-morris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

amateima Nov 6, 2022 • 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

feat: index MerkleDistributor `Claimed` events #110

feat: index MerkleDistributor `Claimed` events #110

amateima Nov 6, 2022 •

edited

Loading