Enhance processing and add caching #1025

aquarat · 2022-04-22T13:07:27Z

This PR extends #1008
and adds asynchronous pre and post processing, which enables some fun use cases, like OAuth2. It also adds a caching utility set, but the caching utilities are not currently used in Airnode. The caching is currently being used by my OAuth2 snippet and I anticipate that it will eventually be used by per-endpoint configurable per requestId API response caching (a requirement for RNG).

OAuth2 has many different flow variations and different providers have vastly different ideas on how to implement OAuth2. This PR allows us to implement arbitrary authentication logic prior to a request being generated (until such time as our formal OAuth2 config is standardised and stable). This allows us to support PoCs in future without having many custom Airnode instances in the wild, which reduces maintenance time keeping forks up to date 🙏 .

sync code follows this format:

const output = {...input, something: 3};

async code should follow this format:

({input, resolve}) => {console.log('hello'); resolve({...input, something: 3})};

resolve is passed through a context object to a vm runtime which the code interacts with.

I've separately played around a bit with a WASM variant of this, but it isn't as powerful due to OS linkage issues. Snippets can be written in a separate project and processed using webpack to create static-like JS snippets which don't have to rely on Airnode's libraries, although the payloads are quite large (530KB for something that uses ethers and axios).

Siegrift

Took an initial look (was curious :p ). I like the vm module from node 💪

packages/airnode-node/src/api/unsafe-evaluate.ts

Siegrift · 2022-04-22T13:57:48Z

packages/airnode-node/src/constants.test.ts

@@ -23,5 +23,6 @@ describe('constants', () => {
    expect(constants.PRIORITY_FEE).toEqual(3120000000);
    expect(constants.BASE_FEE_MULTIPLIER).toEqual(2);
    expect(constants.MAXIMUM_ONCHAIN_ERROR_LENGTH).toEqual(100);
+    expect(constants.PROCESSING_TIMEOUT).toEqual(10_000);


But isn't 10s default too much? Also this timer affects the Airnode cycle and those are not configurable as of now (I think we should discuss this).

packages/airnode-node/src/constants.ts

packages/airnode-utilities/src/caching/index.ts

aquarat · 2022-04-22T14:03:48Z

Took an initial look (was curious :p ). I like the vm module from node muscle

ahahahahaa
It's a draft PR because I had to leave the room I was working in quickly 😆 long story.

Co-authored-by: Emanuel Tesař <[email protected]>

aquarat · 2022-04-22T14:07:21Z

But isn't 10s default too much? Also this timer affects the Airnode cycle and those are not configurable as of now (I think we should discuss this).

The current OAuth2 snippet I'm using (which I can post on Slack if you're curious), takes around 2s to resolve a token (when it needs to refresh the token, often an existing token will be available from the cache in which case it will be much faster, hence why caching is here). But as above I think the timeout should be exposed in config.json in ProcessingSpecification.

…ao/airnode into enhance-processing-and-add-caching

aquarat · 2022-04-22T16:59:21Z

I've kept the timeout on reduce at 10s (via constants.ts) and given the individual code snippets configurable timeouts.
I've switched out eval for vm.runInNewContext.
I need to run a few real-world tests but this should be pretty close to ideal.

aquarat · 2022-04-22T17:01:47Z

But isn't 10s default too much? Also this timer affects the Airnode cycle and those are not configurable as of now (I think we should discuss this).

This is now the total timeout and it shouldn't affect anything else? I think the total parent cycle has 30 seconds of execution time?

aquarat · 2022-04-27T15:37:08Z

This is still pending testing on cloud.
Caching is included for OAuth2 operation.

Siegrift

Looks mostly good to me but I left some comments.

packages/airnode-node/src/api/processing.ts

packages/airnode-node/src/api/unsafe-evaluate.ts

packages/airnode-utilities/src/caching/index.ts

Co-authored-by: Emanuel Tesař <[email protected]>

…essing-and-add-caching

aquarat · 2022-05-01T12:37:11Z

Hopefully I can test this on cloud today.

aquarat · 2022-05-02T17:40:56Z

Works on cloud with the most recent commit. I tried a few different ways of getting this working, ultimately these were the least offensive.

Siegrift

Added a few more comments, but it LGTM overall.

Good job 💪

packages/airnode-node/src/api/unsafe-evaluate.ts

Siegrift · 2022-05-03T07:45:59Z

packages/airnode-node/src/api/unsafe-evaluate.ts

-    const input = _input;
+export const unsafeEvaluate = (input: any, code: string, timeout: number) => {
+  const vmContext = {
+    input,


We stopped passing ethers, I think that one is rather necessary.

Yeah, I think we should also pass all our airnode-* packages.

I fear it will be confusing and harder to document and I do not see an use for using Airnode packages. If you this this is improtant we can keep it, but otherwise I'd just pass ethers and axios.

It's difficult to know what's useful for the future.
At minimum anything can be achieved given the built-in node modules (like I re-implemented the axios requirement using built-in https in the target code snippet). Everything outside of the built in modules is convenience.
I'll remove the airnode-* require/imports :)

packages/airnode-utilities/src/caching/index.test.ts

packages/airnode-utilities/src/caching/index.ts

aquarat · 2022-05-03T16:36:35Z

Done a few more updates, hopefully it is acceptable :P

aquarat · 2022-05-03T16:38:34Z

I will split up nodeModules into nodeModules and extraModules.

Siegrift

Code LGTM 👍 (but I haven't tested manually).

I do not see any more issues just by looking at the code and I believe this was (is) tested by you on the issue you work on.

Siegrift · 2022-05-04T12:57:39Z

packages/airnode-node/src/constants.test.ts

@@ -23,5 +23,6 @@ describe('constants', () => {
    expect(constants.PRIORITY_FEE).toEqual(3120000000);
    expect(constants.BASE_FEE_MULTIPLIER).toEqual(2);
    expect(constants.MAXIMUM_ONCHAIN_ERROR_LENGTH).toEqual(100);
+    expect(constants.PROCESSING_TIMEOUT).toEqual(10_000);


Ignore this. We set 30 seconds timeout and postprocessing can only take 10 seconds. So in worst case the API call has only 20 seconds to finish, but that is still all right.

I also think that we need a timer, because it is possible that there will be an infinite loop or something and we should not wait forever until the preprocessing finishes.

Siegrift · 2022-05-04T12:59:45Z

packages/airnode-utilities/src/caching/index.ts

+      return;
+    }
+
+    // Delete oldest cache entries first


I find it a strange design that we actually can exceed the CACHE_MAX_FILES when adding a new key - but I don't have better suggestion atm.

We would have to sweep on every key addition which would slow things down. We have a lot of overhead @ 20k vs 130k files. It's not "correct" but it is highly unlikely, probably impossible on Lambda, for the cache file count to jump by 110k files in one cycle (Airnode would not be able to process that many requests on Lambda).

packages/airnode-utilities/src/caching/index.ts

Siegrift · 2022-05-04T13:03:59Z

packages/airnode-node/src/constants.test.ts

@@ -23,5 +23,6 @@ describe('constants', () => {
    expect(constants.PRIORITY_FEE).toEqual(3120000000);
    expect(constants.BASE_FEE_MULTIPLIER).toEqual(2);
    expect(constants.MAXIMUM_ONCHAIN_ERROR_LENGTH).toEqual(100);
+    expect(constants.PROCESSING_TIMEOUT).toEqual(10_000);


Btw. your solution handles timeouts well, but I think it would be nice to add a test which tests that. Something like:

const preProcessingSpecifications = [ { environment: 'Node 14' as const, value: 'const output = {...input, from: `ETH`}; while(true);', timeoutMs: 5_000, }, ...

packages/airnode-utilities/src/caching/index.ts

aquarat · 2022-05-05T11:45:29Z

@Siegrift : Regarding

Btw. your solution handles timeouts well, but I think it would be nice to add a test which tests that. Something like:

and

Ignore this. We set 30 seconds timeout and postprocessing can only take 10 seconds. So in worst case the API call has only 20 seconds to finish, but that is still all right.

I assume you mean I should ignore the first comment? I think the timeouts are robust and they definitely work in practice.

Siegrift · 2022-05-05T12:04:52Z

I assume you mean I should ignore the first comment?

Yeah.

aquarat · 2022-05-05T12:23:59Z

Thanks again for your input and patience @Siegrift :)

Enhance processing and add caching

13e3c81

aquarat changed the base branch from master to add-pre-and-post-processing April 22, 2022 13:29

Siegrift reviewed Apr 22, 2022

View reviewed changes

Update packages/airnode-utilities/src/caching/index.ts

dabb964

Co-authored-by: Emanuel Tesař <[email protected]>

aquarat added 2 commits April 22, 2022 18:56

Remove eval in favour of node vm

a8a261a

Merge branch 'enhance-processing-and-add-caching' of github.com:api3d…

bb0c632

…ao/airnode into enhance-processing-and-add-caching

aquarat marked this pull request as ready for review April 27, 2022 15:35

aquarat requested a review from Siegrift April 27, 2022 15:37

Siegrift requested changes Apr 28, 2022

View reviewed changes

aquarat and others added 3 commits April 28, 2022 18:18

Update packages/airnode-node/src/api/processing.ts

d63aa3d

Co-authored-by: Emanuel Tesař <[email protected]>

Update packages/airnode-node/src/api/processing.ts

2130959

Co-authored-by: Emanuel Tesař <[email protected]>

Update packages/airnode-node/src/api/unsafe-evaluate.ts

23cb9c8

Co-authored-by: Emanuel Tesař <[email protected]>

Base automatically changed from add-pre-and-post-processing to master April 29, 2022 07:41

aquarat and others added 6 commits May 1, 2022 13:17

Update packages/airnode-utilities/src/caching/index.ts

35798e4

Co-authored-by: Emanuel Tesař <[email protected]>

Update packages/airnode-utilities/src/caching/index.ts

d990c01

Co-authored-by: Emanuel Tesař <[email protected]>

Polishing of caching

830c0d4

Merge and resolve conflicts

1177ec0

Merge branch 'master' of github.com:api3dao/airnode into enhance-proc…

f8b14c1

…essing-and-add-caching

Add changeset

1c41ae7

Import all built-in node modules to prevent loss due to webpack

30bfd85

Siegrift reviewed May 3, 2022

View reviewed changes

aquarat requested a review from Siegrift May 3, 2022 16:35

Add additional processing imports

451bcb6

aquarat force-pushed the enhance-processing-and-add-caching branch from fe83877 to 451bcb6 Compare May 3, 2022 16:48

Siegrift approved these changes May 4, 2022

View reviewed changes

Use startsWith instead of indexOf in caching key search

0ef99d7

aquarat added 2 commits May 5, 2022 13:45

Merge branch 'master' into enhance-processing-and-add-caching

91b1e22

Remove extra packages from unsafe-evaluate.ts

8d1bbfb

aquarat merged commit 20d98cd into master May 5, 2022

aquarat deleted the enhance-processing-and-add-caching branch May 5, 2022 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance processing and add caching #1025

Enhance processing and add caching #1025

aquarat commented Apr 22, 2022 •

edited

Loading

Siegrift left a comment

Siegrift Apr 22, 2022

aquarat commented Apr 22, 2022

aquarat commented Apr 22, 2022 •

edited

Loading

aquarat commented Apr 22, 2022 •

edited

Loading

aquarat commented Apr 22, 2022

aquarat commented Apr 27, 2022

Siegrift left a comment

aquarat commented May 1, 2022

aquarat commented May 2, 2022 •

edited

Loading

Siegrift left a comment

Siegrift May 3, 2022

aquarat May 3, 2022

Siegrift May 4, 2022

aquarat May 5, 2022

aquarat commented May 3, 2022

aquarat commented May 3, 2022 •

edited

Loading

Siegrift left a comment

Siegrift May 4, 2022

Siegrift May 4, 2022

aquarat May 5, 2022

Siegrift May 4, 2022

aquarat commented May 5, 2022 •

edited

Loading

Siegrift commented May 5, 2022

aquarat commented May 5, 2022

Enhance processing and add caching #1025

Enhance processing and add caching #1025

Conversation

aquarat commented Apr 22, 2022 • edited Loading

Siegrift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aquarat commented Apr 22, 2022

aquarat commented Apr 22, 2022 • edited Loading

aquarat commented Apr 22, 2022 • edited Loading

aquarat commented Apr 22, 2022

aquarat commented Apr 27, 2022

Siegrift left a comment

Choose a reason for hiding this comment

aquarat commented May 1, 2022

aquarat commented May 2, 2022 • edited Loading

Siegrift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aquarat commented May 3, 2022

aquarat commented May 3, 2022 • edited Loading

Siegrift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aquarat commented May 5, 2022 • edited Loading

Siegrift commented May 5, 2022

aquarat commented May 5, 2022

aquarat commented Apr 22, 2022 •

edited

Loading

aquarat commented Apr 22, 2022 •

edited

Loading

aquarat commented Apr 22, 2022 •

edited

Loading

aquarat commented May 2, 2022 •

edited

Loading

aquarat commented May 3, 2022 •

edited

Loading

aquarat commented May 5, 2022 •

edited

Loading