Contribution Count Shows Different Numbers - fixed #443 #460

aj-maz · 2021-10-29T11:22:56Z

The root of the bug was the "first" parameter in the GraphQL query which has a default number of 100 and was not handled:
{ grantDonations(where: {lastUpdatedBlockNumber_gte: ${fromBlock}, lastUpdatedBlockNumber_lte: ${blockNumber}}) { grantId tokenIn donationAmount from hash rounds lastUpdatedBlockNumber } }

so whenever the length of the contributions would go more than 100, the system would act inconsistently. I created a recursive function that will continue to fetch contributions until there is no more left.

It only works if the cached data is properly set before "fromBlock" which is not unless there are less than 100 contributions So do the hard refresh and remove the cached data

apbendi · 2021-10-29T15:20:02Z

app/src/utils/data/contributions.ts

+                const json = await res.json();
+
+                if (json.data.grantDonations.length) {
+                  return await fetchUntilAll(SUBGRAPH_URL, before, skip + 1);


Fixes needed:

merge json response with before before recursive call

increment indexdb number to bust cache

Here's where to increment the version

dgrants/app/src/utils/constants.ts

Lines 43 to 44 in 01f6205

// we can bump this version number to bust the users cache

version: 1,

aj-maz · 2021-10-29T17:59:53Z

I contribute to 6 grants on my own wallet and for other users everything was fine, but the contributor user only fetched 2 contributions. This PR is still an improvement and will fix the problem for most users, but for fixing #443 completely I need to find the root of this problem as well. Maybe we should create a new issue for it.

mds1 · 2021-10-29T20:06:58Z

app/src/utils/constants.ts

@@ -41,7 +41,7 @@ export const DefaultForageConfig: LocalForageConfig = {
  // this store should be used for any on-chain/off-chain data but never user data (as we might clear it without notice)
  name: 'dGrants',
  // we can bump this version number to bust the users cache
-  version: 1,
+  version: 3,


Why 3 and not 2?

I wasn't sure what should I put, thought maybe some other PR is going to use 2.

There are no other open PRs changing this, so we should use 2 🙂

Suggested change

version: 3,

version: 2,

mds1 · 2021-10-29T20:09:56Z

app/src/utils/data/contributions.ts

@@ -56,7 +56,7 @@ export async function getContributions(

              const limit = 100;

-              const fetchUntilAll = async (SUBGRAPH_URL: string, before = [], skip = 0): Promise<any[]> => {
+              const fetchUntilAll = async (SUBGRAPH_URL: string, before: any[] = [], skip = 0): Promise<any[]> => {


Ideally we should have stronger typing than any, but we can save that for the PR that handles #463

Now we use type ContributionSubgraph instead of any.

mds1 · 2021-10-29T20:11:39Z

I contribute to 6 grants on my own wallet and for other users everything was fine, but the contributor user only fetched 2 contributions. This PR is still an improvement and will fix the problem for most users, but for fixing #443 completely I need to find the root of this problem as well. Maybe we should create a new issue for it.

@Ajand code-wise this looks good, but I'm not sure what exactly you mean here? Which users is this working for and which is it only fetching 2 contributions for?

aj-maz · 2021-10-29T20:14:37Z

I contribute to 6 grants on my own wallet and for other users everything was fine, but the contributor user only fetched 2 contributions. This PR is still an improvement and will fix the problem for most users, but for fixing #443 completely I need to find the root of this problem as well. Maybe we should create a new issue for it.

@Ajand code-wise this looks good, but I'm not sure what exactly you mean here? Which users is this working for and which is it only fetching 2 contributions for?

Let's assume there are 3 users. user 1 will contribute to 10 grants. for user2 and user3, everything is fine, but for user1 we gonna get inconsistent results.

metafraction · 2021-10-29T20:56:33Z

agree with @mds1 comments, so once those are resolved (and approved), I'll approve
good work!

aj-maz · 2021-10-31T16:40:59Z

Wrong commit message. There is nothing here for #469. I meant #460.

aj-maz · 2021-11-01T09:41:30Z

There was two problems with #470:

The grant donation function would start a race with itself and would override the data it wanted to save so if in a block there would be more than one contribution some of them would not be saved even though they were fetched. I solved that issue by creating an async function call wait(timeout) and using a closure called timoutManager to put a timeout so the next call would wait so the last call could save its data.
For contributor users for some reason the listener processor would be called 2 times simultaneously so even the in-memory closure wouldn't be able to stop the race. So I added a random timeout in addition to a normal timeout and used a set to prevent processing the same event multiple times.

This approach works fine for now but if we're going to scale ( for example if we have 50 contributions in each block ) it doesn't work. We need to create a queue while processing the events and saving all contributions at once after processing.

Also, it would be nice to find the root of double processing for contributor users so could stop it. Since I'm using a random number there is still a slim chance for some contributors to have some issues.

phutchins · 2021-11-05T13:59:39Z

Whats left here @Ajand ? Is this read for final review? @mk1r thoughts?

aj-maz · 2021-11-05T14:05:36Z

Hi @phutchins .
I think it's finished. It's not the ultimate best solution but it's a pretty good solution for now and I believe it works unless we get to Facebook level scale. I guess we just need @gdixon approval as @mds1 requested.

metafraction · 2021-11-08T15:37:16Z

@phutchins yes looks good to me, are all of @mds1 concerns resolved? @Ajand

metafraction

good job!

fixed dcgtc#443

557627a

aj-maz changed the title ~~fixed #443~~ Contribution Count Shows Different Numbers - fixed #443 Oct 29, 2021

apbendi mentioned this pull request Oct 29, 2021

Fix Subgraph Call Pagination #463

Closed

apbendi reviewed Oct 29, 2021

View reviewed changes

before merged with json

63e1389

mds1 reviewed Oct 29, 2021

View reviewed changes

aj-maz closed this Oct 29, 2021

aj-maz reopened this Oct 29, 2021

type and version fixedfor dcgtc#469

346f725

aj-maz mentioned this pull request Oct 31, 2021

Inconsistency when user contribute to more than one grant #470

Closed

fixed dcgtc#470

749f2ae

mds1 requested a review from gdixon November 1, 2021 13:20

apbendi requested review from mds1, thelostone-mc and metafraction November 5, 2021 15:47

$metafraction$

metafraction approved these changes Nov 8, 2021

View reviewed changes

This was linked to issues Nov 8, 2021

Contribution Count Shows Different Numbers #443

Closed

Inconsistency when user contribute to more than one grant #470

Closed

phutchins closed this Nov 8, 2021

gdixon mentioned this pull request Nov 10, 2021

Fixes performance issues #493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contribution Count Shows Different Numbers - fixed #443 #460

Contribution Count Shows Different Numbers - fixed #443 #460

aj-maz commented Oct 29, 2021 •

edited

Loading

apbendi Oct 29, 2021 •

edited by aj-maz

Loading

mds1 Oct 29, 2021

aj-maz commented Oct 29, 2021

mds1 Oct 29, 2021

aj-maz Oct 29, 2021

mds1 Oct 29, 2021

aj-maz Oct 31, 2021

mds1 Oct 29, 2021

aj-maz Oct 31, 2021

mds1 commented Oct 29, 2021

aj-maz commented Oct 29, 2021

metafraction commented Oct 29, 2021 •

edited

Loading

aj-maz commented Oct 31, 2021

aj-maz commented Nov 1, 2021

phutchins commented Nov 5, 2021

aj-maz commented Nov 5, 2021

metafraction commented Nov 8, 2021

$@metafraction$ metafraction left a comment

	// we can bump this version number to bust the users cache
	version: 1,

Contribution Count Shows Different Numbers - fixed #443 #460

Contribution Count Shows Different Numbers - fixed #443 #460

Conversation

aj-maz commented Oct 29, 2021 • edited Loading

apbendi Oct 29, 2021 • edited by aj-maz Loading

Choose a reason for hiding this comment

mds1 Oct 29, 2021

Choose a reason for hiding this comment

aj-maz commented Oct 29, 2021

mds1 Oct 29, 2021

Choose a reason for hiding this comment

aj-maz Oct 29, 2021

Choose a reason for hiding this comment

mds1 Oct 29, 2021

Choose a reason for hiding this comment

aj-maz Oct 31, 2021

Choose a reason for hiding this comment

mds1 Oct 29, 2021

Choose a reason for hiding this comment

aj-maz Oct 31, 2021

Choose a reason for hiding this comment

mds1 commented Oct 29, 2021

aj-maz commented Oct 29, 2021

metafraction commented Oct 29, 2021 • edited Loading

aj-maz commented Oct 31, 2021

aj-maz commented Nov 1, 2021

phutchins commented Nov 5, 2021

aj-maz commented Nov 5, 2021

metafraction commented Nov 8, 2021

metafraction left a comment

Choose a reason for hiding this comment

aj-maz commented Oct 29, 2021 •

edited

Loading

apbendi Oct 29, 2021 •

edited by aj-maz

Loading

metafraction commented Oct 29, 2021 •

edited

Loading

$@metafraction$ metafraction left a comment