Implementation

[juliangruber]

This implements revision D of https://spacemeridian.notion.site/Meridian-Off-chain-scheduled-rewards-f9480ef009464ecfaf02a4f752cc4d70.

I decided to use Redis as the data store, because:

• It's easy to install and run
• Fly supports it out of the box
• It is less complex than postgres
• Redis data structures made this straight forward
• This service will be deprecated asap

[bajtos]

I see you made changes while I was reviewing the PR, I will review them after posting this comment.

The change was just setting SENTRY_ENVIROMENT for Fly.io, that part LGTM.

[bajtos]

Almost there!

Do you have any concerns about the the growing storage requirements for logs? I did a quick calculation, we will produce ~60MB per day if we have 5k participants.

Redis supports TTL-based expiration. Can we use it for log entries? I think TTL of 30 days is more than enough for what we need.

[bajtos]

Doesn't node:test provide an API for grouping tests to test suites? I know they provide both describe and it, what is the counterpart for describe? Maybe suite?

[juliangruber]

I'm not sure I follow, isn't t.test() grouping tests as well?

[juliangruber]

This is the tap way of defining a subtest

[juliangruber]

in node:test, describe() is an alias of suite(): https://nodejs.org/api/test.html#describe-and-it-aliases.

I'm not sure when to use suite() vs test(), do you know how it could improve this test suite?

[bajtos]

I would use suite for grouping related tests and test for individual tests, that's how I am using describe and it.

Basically, only suites are allowed to contain nested things (tests or suites).

suite('scheduled rewards', () => {
  test('empty scheduled rewards', async t => {
    // ...
  })

  test('ignore burner address', async t => {
    // ...
  })

  // etc.
})

This way, you also do not need to await t.test.

See the example code snippet provided in Snapshot Testing:

// test.js
suite('suite of snapshot tests', () => {
  test('snapshot test', (t) => {
    t.assert.snapshot({ value1: 1, value2: 2 });
    t.assert.snapshot(5);
  });
});

[juliangruber]

I understand, thank you, but why is it better?

I assume it's better because you can filter tests better? I never use this functionality so I don't mind. Would you like me to rework this using suite()?

[juliangruber]

Basically, only suites are allowed to contain nested things (tests or suites).

Please see https://nodejs.org/api/test.html#contexttestname-options-fn, node test doesn't have this limitation

[juliangruber]

Switched to suites in

• 84a7854
• e2fbde8

[bajtos]

I understand, thank you, but why is it better?

I find the test code easier to read this way because it's clear to me which parts define the structure and which parts are actually executed as tests.

I assume it's better because you can filter tests better?

That's very likely! I don't have any experience with your style of structuring tests, so I cannot tell how it affects test filtering.

I never use this functionality so I don't mind. Would you like me to rework this using suite()?

That would be great, thank you! 🙏🏻

Switched to suites

😍

[juliangruber]

Almost there!

Do you have any concerns about the the growing storage requirements for logs? I did a quick calculation, we will produce ~60MB per day if we have 5k participants.

Redis supports TTL-based expiration. Can we use it for log entries? I think TTL of 30 days is more than enough for what we need.

Good point, I've limited logs to ~30 days.

Redis doesn't (easily) support expiration of list entries, therefore I'm capping the list based on an estimation of how many entries we have after 30 days:

3 rounds per hour * 24 hours * 30 days * 5000 participants