e2e regression tests: HTTP caching proxy, new cases #633

mitjat · 2024-02-09T05:42:32Z

This PR expands our e2e testing capabilities; the motivation is the upcoming support for partial verifications in Sourcify.

The changes are:

A new caching HTTP proxy is introduced, based on our existing KVStore. It allows e2e tests to cache HTTP responses (e.g. from sourcify) on an initial run, and then replay them deterministically on every subsequent run.
- Small refactor: KVStore and the new proxy are moved to a dedicated package.
The evmverifier analyzer is now included in e2e tests, using the new proxy.
- The PR also includes two new db__* test cases that aim to capture the "derived" columns of txs and events, notably the ABI-parsing results.
- Despite that, there are no diffs between before and after evmverifier. Our current e2e block range does not engage with any contracts that are fully verified 😬
- This will hopefully change when Andrew7234/e2e extension #627 lands and re-introduces a Damask range, and will certainly change when we support partially-verified contracts (though minimally; there's only 1 partially verified contract that this e2e block range interacts with.
The item analyzer sleep logic has changed a little. The old stop_on_empty_queue has been replaced with a duration-based flag ("exit only if the queue has been empty for N seconds"). This is helpful for item analyzers that interact with each other.

ptrus · 2024-02-09T09:07:49Z

cmd/analyzer/analyzer.go

@@ -512,6 +526,16 @@ func (a *Service) Start() {
 	ctx, cancelAnalyzers := context.WithCancel(context.Background())
 	defer cancelAnalyzers() // Start() only returns when analyzers are done, so this should be a no-op, but it makes the compiler happier.

+	// Start helpers.
+	for _, helper := range a.helpers {


Should probably also try to gracefully Shutdown() the server helpers in received interrupt, shutting down part. And call Close() on the servers in cleanup.

... not sure it's worth it for these proxies that are intended for e2e testing only and will be shut down simply by killing the whole binary.

But it's good to encourage nice clean code :). Added both, thank you.

Today's end-to-end tests are tomorrow's production features 😁

ptrus · 2024-02-09T09:10:33Z

cmd/analyzer/analyzer.go

@@ -149,6 +151,7 @@ func wipeStorage(cfg *config.StorageConfig) error {
 type Service struct {
 	analyzers         []SyncedAnalyzer
 	fastSyncAnalyzers []SyncedAnalyzer
+	helpers           []*http.Server


Should this be just named (caching) proxies, at least while caching proxies are the only type of helpers there is?

👍 renamed here and related variables throughout the file.

pro-wh · 2024-02-09T18:17:24Z

accessing this cache over HTTP, can you talk about this

in existing code that does similar, there's a Go interface and a caching implementation of that

analyzer/item/item.go

Andrew7234 · 2024-02-21T14:03:03Z

analyzer/evmverifier/evmverifier.go

+		// NOTE: We upsert rather than update; if the contract is not in the db yet, UPDATE would ignore the
+		//       contract and this analyzer would keep retrying it on every iteration.


Hm is there a case where the verifier would upsert rather than update? The GetItems query fetches candidates from the chain.evm_contracts table originally

An insert can happen if sourcify has a contract that we haven't encountered yet, e.g. because we're scanning just a subset of blocks. GetItems no longer limits the items to chain.evm_contracts addresses as of #634 (which is not merged; I accidentally merged it into this branch and then undid that, but I'll duplicate the PR to merge into main once this PR goes in). The reasons for the change are pretty lightweight: we now don't have to worry about what happens as the number of contracts grows, and I found the new flow easier to understand, especially with the added partial/full verification.

tests/e2e_regression/e2e_config_2.yml

tests/e2e_regression/run.sh

pro-wh

can we switch to a mockable http client later

mitjat · 2024-02-22T04:44:51Z

can we switch to a mockable http client later

Yeah I'm totally OK with that. Right now the http proxy is only being used for sourcify, and it's a low enough number of requests that we could mock them more explicitly. FWIW, the caching proxy achieves pretty much the same effect, with the debugging downside that it's a lot harder to see the cached/mocked responses.

pro-wh · 2024-02-22T18:07:50Z

what's the item analyzer exit delay for? is there a new test that relies on it?

new config api can't say to do what we did before, you instead have to set a very small delay

mitjat · 2024-02-22T21:15:27Z

what's the item analyzer exit delay for? is there a new test that relies on it?

Yeah the e2e. We ideally would want to run in 3 stages ...
#633 (comment)

evm_abi_emerald:               { stop_if_queue_empty_for: 10s } # Give evm_contract_verifier time to fetch ABIs first. The 10s has been enough in practice, but might need to be tuned in the future, especially if the caching proxy has an empty cache.

new config api can't say to do what we did before, you instead have to set a very small delay

True. I worried about it a little at first, but then decided it's just an aesthetic problem, not a functional one. And it only applies to testing. So screw it.

mitjat · 2024-02-23T02:44:26Z

After rebasing on top of #627 and debugging, I realized that our internal Damask Emerald node is currently having issues. I therefore disabled the Emerald part of the new Damask e2e test. This version of this PR still makes e2e tests usefully more comprehensive to help with merging #634.

This reverts commit 91dd3b8.

mitjat requested review from Andrew7234, pro-wh and ptrus as code owners February 9, 2024 05:42

mitjat changed the title ~~e2e tests: HTTP caching proxy, new cases~~ e2e regression tests: HTTP caching proxy, new cases Feb 9, 2024

mitjat force-pushed the mitjat/e2e-cache-http branch 2 times, most recently from a7f90b3 to c7ff623 Compare February 9, 2024 07:35

ptrus reviewed Feb 9, 2024

View reviewed changes

mitjat force-pushed the mitjat/e2e-cache-http branch 4 times, most recently from 4d4be77 to c3f6a8f Compare February 16, 2024 01:03

mitjat requested a review from ptrus February 16, 2024 01:04

mitjat force-pushed the mitjat/e2e-cache-http branch 3 times, most recently from 1075341 to 57204f9 Compare February 16, 2024 01:31

mitjat mentioned this pull request Feb 21, 2024

properly evmMarshal evm_log_params #643

Merged

Andrew7234 reviewed Feb 21, 2024

View reviewed changes

pro-wh reviewed Feb 21, 2024

View reviewed changes

mitjat force-pushed the mitjat/e2e-cache-http branch from 57204f9 to 9e5ad3d Compare February 22, 2024 05:36

mitjat requested review from Andrew7234 and pro-wh February 22, 2024 05:36

mitjat force-pushed the mitjat/e2e-cache-http branch from 9e5ad3d to 6622cc4 Compare February 22, 2024 05:52

ptrus approved these changes Feb 22, 2024

View reviewed changes

mitjat force-pushed the mitjat/e2e-cache-http branch 2 times, most recently from cb3c188 to 458d2ca Compare February 22, 2024 20:35

e2e: new use case: runtime txs, events

5c72ea8

mitjat added 11 commits February 22, 2024 14:44

kvstore: Extract into new package

f8f3dec

Implement caching_http_proxy

e5e6073

cmd/analyzer: run caching_http_proxy as helper

3a935e5

analyzer/item: Do not sleep on start

8a43600

analyzer/item: Do not exit immediately on empty queue

87da639

analyzer/evmverifier: Use upsert, allow short query intervals

a9be27d

rename helpers -> cachingProxies

6c5721c

analyzers: close caching proxies on termination

abbd836

e2e_regression: cache: Do not track index files in git

91dd3b8

e2e: accept-e2e-regression: Do nothing if there are no outputs.

12458fe

e2e: Update outputs

aa761a4

mitjat force-pushed the mitjat/e2e-cache-http branch from 458d2ca to aa761a4 Compare February 23, 2024 02:41

Revert "e2e_regression: cache: Do not track index files in git"

534a3b1

This reverts commit 91dd3b8.

Andrew7234 approved these changes Feb 23, 2024

View reviewed changes

mitjat merged commit 908a841 into main Feb 23, 2024
10 checks passed

mitjat deleted the mitjat/e2e-cache-http branch February 23, 2024 02:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

e2e regression tests: HTTP caching proxy, new cases #633

e2e regression tests: HTTP caching proxy, new cases #633

mitjat commented Feb 9, 2024

ptrus Feb 9, 2024

mitjat Feb 15, 2024

ptrus Feb 22, 2024

ptrus Feb 9, 2024

mitjat Feb 15, 2024

pro-wh commented Feb 9, 2024

Andrew7234 Feb 21, 2024

mitjat Feb 22, 2024

pro-wh left a comment

mitjat commented Feb 22, 2024

pro-wh commented Feb 22, 2024

mitjat commented Feb 22, 2024

mitjat commented Feb 23, 2024

		// NOTE: We upsert rather than update; if the contract is not in the db yet, UPDATE would ignore the
		// contract and this analyzer would keep retrying it on every iteration.

e2e regression tests: HTTP caching proxy, new cases #633

e2e regression tests: HTTP caching proxy, new cases #633

Conversation

mitjat commented Feb 9, 2024

ptrus Feb 9, 2024

Choose a reason for hiding this comment

mitjat Feb 15, 2024

Choose a reason for hiding this comment

ptrus Feb 22, 2024

Choose a reason for hiding this comment

ptrus Feb 9, 2024

Choose a reason for hiding this comment

mitjat Feb 15, 2024

Choose a reason for hiding this comment

pro-wh commented Feb 9, 2024

Andrew7234 Feb 21, 2024

Choose a reason for hiding this comment

mitjat Feb 22, 2024

Choose a reason for hiding this comment

pro-wh left a comment

Choose a reason for hiding this comment

mitjat commented Feb 22, 2024

pro-wh commented Feb 22, 2024

mitjat commented Feb 22, 2024

mitjat commented Feb 23, 2024