Add live scheduled rewards on current day. Closes #1677

[juliangruber]

Closes #1677

[bajtos]

So many new Date instances! Have you considered using something like this?

const yesterday = new Date(Date.now() - 24 * 3600_000)

[juliangruber]

I don't have a strong opinion on this. To me the current version is better because

• performance doesn't matter here at all
• it uses Date APIs that work on a day level, not on a millisecond level
• it doesn't use magics or constants that you need to remember (3600s = 1h)

What is your concern with the current version? Or are you saying it's difficult to read because of the many date instances?

[bajtos]

I find Date APIs confusing - some values are zero-based (months), and some are one-based (day of the month). getDate returns the day of the month, and getTime returns milliseconds since the Unix epoch. I always have to check the docs to recall what each method does.

Anyhow, this is not a big deal. The variable name clearly communicates what we are calculating, the rest is an implementation detail.

We can keep the current version.

[juliangruber]

I understand! I also feel that way about 0 vs 1 based Date functions, but I feel safe here since we're passing a value + 1, after reading it from Date, so whether the function is 0 based or 1 based doesn't make a difference

[bajtos]

I am confused about this part, could you please explain at high level how is this fixing the problem?

Several lines above, I see that you call https://stats.filspark.com/participant/${address}/scheduled-rewards with to=${yesterday}.

That means we will never fetch rewards for today. The code here seems to apply yesterday's rewards for today. Shouldn't we fetch today's reward from somewhere?

[juliangruber]

the goal is to show live scheduled rewards of today, not total rewards received. Therefore, if there's no entry for today, I add a new entry and set its total rewards to the ones from the day before.

Then, below in line 170, we set the scheduled rewards of the most recent entry to the live value from the contract.

[bajtos]

I see. Would you mind capturing this information in a code comment?

if (
      !todayRewards ||
      new Date(todayRewards.timestamp).getDate() !== new Date().getDate() ||
      new Date(todayRewards.timestamp).getMonth() !== new Date().getMonth() ||
      new Date(todayRewards.timestamp).getFullYear() !== new Date().getFullYear()
    ) {
      // If there's no entry for today, add a new entry copying data from yesterday.
      // We can use rewards from yesterday because we will change that value
      // later to the live value obtained from the smart contract
      todayRewards = {

[juliangruber]

That's not what happening though: We're setting totalRewardsReceived from yesterday and will not update it afterwards. What we're doing is we're setting totalScheduledRewards to 0 and then afterwards update that with the live value from the contract.

We don't have total rewards as a live value in the contract.

[bajtos]

I am struggling to understand the fix fully, but I also don't have the appetite to study the code closely to improve my understanding.

Let's get this shipped and see how it works in practice.

[juliangruber]

I think the main disconnect is that this is setting a live value for scheduled rewards, not total rewards. Happy to walk through it if you would like