Add smoketest to e2e tests by piersy · Pull Request #261 · celo-org/op-geth

piersy · 2024-10-18T18:19:45Z

Adds an e2e smoketest that sends value transfer, contract interaction
and contract creation transactions for all of the valid transaction types.

It also verifies that deprecated transactions (celo legacy & cip42) are
not supported.

e2e_test/js-tests/test_viem_smoketest.mjs

karlb · 2024-10-21T06:23:52Z

e2e_test/js-tests/test_viem_smoketest.mjs

+	// This test is failing because the produced transaction is of type cip64.
+	// I guess this is a problem with the viem internals.
+	it.skip("cip42 not supported", async () => {
+		const type = "cip42";
+		await verifyTypedTransactions(type);
+	}).timeout(10_000);


Interesting and I think your explanation is correct. We could try running the same test against Celo L1 to make sure it's not a change in the chain behaviour that is causing this.

@piersy just to get the explanation right - viem with the new l2 alfajores definitions does not seem to know about "cip42" types and instead produces a cip64 transaction, that then fails the assertion?

I would classify this as a viem bug, since it shouldn't even allow the deprecated `cip42' type and fail early on unknown types (at least in my expectation).

yeah, I'd classify it as a viem bug, i will raise it with the dev tooling team

However viem still needs to be able to send cip42 txs to the celo l1, so it does still need that functionality.

Does it? It could just send CIP-64 without missing any features.

Unless people really want to set the gatewayFee and gatewayFeeRecipient! I'll check with the devtooling team to see what the expected approach for viem is.

You can't set those to non-zero values anymore (celo-blockchain will reject the tx otherwise), so nothing is lost.

Ok, was unaware!

@karlb @ezdac I've moved this test into the smoketest_unsupported_txs golang test. Where we can be sure that we are sending an invalid transaction, and therefore ensure that the server correctly rejects it.

e2e_test/js-tests/test_viem_smoketest.mjs

karlb · 2024-10-21T06:32:59Z

e2e_test/js-tests/test_viem_smoketest.mjs

+			assert.fail("Managed to send unsupported legacy tx with fee currency");
+	}).timeout(10_000);
+
+	it.only("legacy create tx with fee currency not supported", async () => {


I'm not sure if it's worth having this test in addition to the previous one. Chances that tx types behave totally differently between send and create are low and we already test plenty of both in other tests with having to repeat them.

Hey @karlb I've removed these tests from the viem test and added them in golang since its easier to construct known unsupported transactions in golang, in the golang version I only test send transactions.

e2e_test/test_smoketest.sh

ezdac

Looks good so far! I added some suggestions and questions regarding unsupported tx types.
This branch requires a rebase against celo10.

e2e_test/js-tests/test_viem_smoketest.mjs

ezdac · 2024-10-21T23:08:06Z

e2e_test/js-tests/test_viem_smoketest.mjs

+	// This test is failing because the produced transaction is of type cip64.
+	// I guess this is a problem with the viem internals.
+	it.skip("cip42 not supported", async () => {
+		const type = "cip42";
+		await verifyTypedTransactions(type);
+	}).timeout(10_000);


@piersy just to get the explanation right - viem with the new l2 alfajores definitions does not seem to know about "cip42" types and instead produces a cip64 transaction, that then fails the assertion?

I would classify this as a viem bug, since it shouldn't even allow the deprecated `cip42' type and fail early on unknown types (at least in my expectation).

e2e_test/js-tests/test_viem_smoketest.mjs

.github/workflows/ci.yaml

karlb · 2024-10-22T13:23:06Z

Why is the CI workflow with the tests not run for the last commits? The workflow file looks fine to me.

piersy · 2024-10-22T13:33:15Z

Why is the CI workflow with the tests not run for the last commits? The workflow file looks fine to me.

Hey @karlb, no idea I only made one change to CI which was to run these tests on alfajores. Maybe because the automatic base change or the conflicts it now has?

Update: The workflow seemed to get run again after rebasing this on celo10, perhaps it was the existence of conflicts between this and celo10 that caused the workflow to not get run.

The smoketest tries all types of transaction for both sends and creates.

Chose a timeout of 16 seconds since the first two tests were taking 8 seconds on Alfajores.

Note that in many cases we were able to remove explicit use of account in transaction creattion because it is "hoisted" in the walletClient. Also removed chain-id and ACC_PRIVKEY as arguments from send_tx.mjs because they are controlled by env vars, which are picked up by the mjs file.

e2e_test/js-tests/test_viem_smoketest.mjs

e2e_test/test_smoketest.sh

Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

Due to the way that viem is structured it is a bad idea for us to block transactions based on the supplied tx attributes, since they may be supported by downstream components such as optimism's custom viem functionality. So rather than trying to test unsupported txs via viem, we now test via golang which allows us full control of the sent transaction, thereby ensuring that rejection is happening at the node level. This commit also extends the error message for deprecated transactions to distinguish between celo legacy and plain legacy transactions.

Adds an e2e smoketest that sends value transfer, contract interaction and contract creation transactions for all of the valid transaction types. It also verifies that deprecated transactions (celo legacy & cip42) are not supported. Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

…upported (#261)

Adds an e2e smoketest that sends value transfer, contract interaction and contract creation transactions for all of the valid transaction types. It also verifies that deprecated transactions (celo legacy & cip42) are not supported. Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

…upported (#261)

Adds an e2e smoketest that sends value transfer, contract interaction and contract creation transactions for all of the valid transaction types. Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

Adds an e2e smoketest that sends value transfer, contract interaction and contract creation transactions for all of the valid transaction types. It also verifies that deprecated transactions (celo legacy & cip42) are not supported. Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

…d_tx.mjs (#261) Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

piersy requested review from ezdac and karlb October 18, 2024 18:19

karlb reviewed Oct 21, 2024

View reviewed changes

Base automatically changed from piersy/run-e2e-tests-on-alfajores to celo10 October 21, 2024 10:58

karlb approved these changes Oct 21, 2024

View reviewed changes

e2e_test/test_smoketest.sh Outdated Show resolved Hide resolved

ezdac requested changes Oct 21, 2024

View reviewed changes

karlb reviewed Oct 22, 2024

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

piersy force-pushed the piersy/smoketest branch from 30c3afa to 7fb90b5 Compare October 22, 2024 13:52

piersy added 13 commits October 22, 2024 15:06

Add smoketest to e2e tests

81beb5b

The smoketest tries all types of transaction for both sends and creates.

Fix erroneous timeout

013cf7e

Remove use of only (left over from local debugging)

54b3a9b

Add a sleep to prevent spamming the server

a65314e

Set smoketest timeout globally

ab7836f

Chose a timeout of 16 seconds since the first two tests were taking 8 seconds on Alfajores.

Add smart contract interactions to smoke test

e89bad1

Use writeContract instead of sendTransaction

c1cfab5

Remove trailing space

34ea9bd

Correct formatting

5158b37

Move assertions next to fetching actions

d8e7235

Parameterise viem smoketests

3955e5f

Fix just enough gas test

1f87012

piersy force-pushed the piersy/smoketest branch from 7fb90b5 to 1f87012 Compare October 22, 2024 14:33

Update skipped test to not reference missing function

418e542

aaronmgdr reviewed Oct 22, 2024

View reviewed changes

e2e_test/js-tests/test_viem_smoketest.mjs Show resolved Hide resolved

karlb reviewed Oct 23, 2024

View reviewed changes

e2e_test/test_smoketest.sh Outdated Show resolved Hide resolved

piersy and others added 3 commits October 23, 2024 13:11

Update e2e_test/test_smoketest.sh

fcf6c8e

Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

Don't run golang smoketest run with usual go tests

1960e67

Kourin1996 pushed a commit that referenced this pull request Jul 30, 2025

Verifies that deprecated transactions (celo legacy & cip42) are not s…

ac883a1

…upported (#261)

Kourin1996 pushed a commit that referenced this pull request Jul 30, 2025

Verifies that deprecated transactions (celo legacy & cip42) are not s…

55d607b

…upported (#261)

Kourin1996 pushed a commit that referenced this pull request Aug 1, 2025

e2e(fee_currency): Import viem_setup.mjs instead of setup viem in sen…

22e684f

…d_tx.mjs (#261) Co-authored-by: Karl Bartel <karl.bartel@clabs.co>

Conversation

piersy commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ezdac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

karlb commented Oct 22, 2024

Uh oh!

piersy commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

piersy commented Oct 18, 2024 •

edited

Loading

piersy commented Oct 22, 2024 •

edited

Loading