Run e2e tests on alfajores by piersy · Pull Request #258 · celo-org/op-geth

piersy · 2024-10-16T21:40:52Z

Modifies our e2e tests to be able to run on alfajores if the environment variable NETWORK is set to 'alfajores'.
Also adds a CI step to run them against alfajores.

Note that the test_fee_currency... tests are skipped because they require the ability to deploy new fee currencies, which we probably don't want to be doing on a live network as it could cause issues for people, and we also don't want to expose the credentials to do so.

As opposed to sending it async, this ensures that we don't end up with an invalid nonce on the next transaction.

socket-security · 2024-10-16T21:41:08Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/viem@2.21.27	Transitive: environment, network	`+11`	18.3 MB	awkweb, jmoxey

🚮 Removed packages: npm/viem@2.9.15

View full report↗︎

piersy · 2024-10-17T09:42:24Z

e2e_test/js-tests/test_viem_tx.mjs

+	it("send fee currency tx with just high enough gas price mine", async () => {
+		const rate = await getRate(process.env.FEE_CURRENCY);
 		const block = await publicClient.getBlock({});
-		// Actually, the base fee will be a bit lower next block, since our blocks
-		// are always mostly empty. But the difference will be much less than the
-		// exchange rate of 2, so the test will still check that we take the fee
-		// currency into account everywhere.
-		const maxFeePerGas = block.baseFeePerGas / 2n;
+		const maxFeePerGas = rate.toFeeCurrency(block.baseFeePerGas)+2n;
 		const request = await walletClient.prepareTransactionRequest({
 			account,
 			to: "0x00000000000000000000000000000000DeaDBeef",
 			value: 2,
-			gas: 90000,
-			feeCurrency: process.env.FEE_CURRENCY2,
-			maxFeePerGas,
-			maxPriorityFeePerGas: 1n,
+			gas: 171000,
+			feeCurrency: process.env.FEE_CURRENCY,
+			maxFeePerGas: maxFeePerGas,
+			maxPriorityFeePerGas: 2n,


Hey @karlb, I'm thinking probably my modifications here don't make sense because it seems that it is important for this test that the fee per gas for the fee currency than the native currency, and I've modified it to use USDC which is the other way around.

It seems that the main point of this is that the fee should be too low for native currency, but if the node converts it then it is sufficient. But for USDC it should actually be fine, since the exchange rate results in requiring a lower value for the base-fee compared to when it is denominated in Celo.

I will leave as is, with a comment that we may want to change this if the exchange rate changes significantly.

Without this it can hang for about 12 minutes!

ezdac

I didn't manage to look at everything in detail yet.
But looks very good so far - this feature is much appreciated.

For future changes: I think we should add more fee-currencies registered in the directory and test this for multiple currencies.

e2e_test/js-tests/test_viem_tx.mjs

ezdac · 2024-10-17T22:22:42Z

e2e_test/js-tests/test_viem_tx.mjs

+	it("send fee currency tx with just high enough gas price mine", async () => {
+		const rate = await getRate(process.env.FEE_CURRENCY);
 		const block = await publicClient.getBlock({});
-		// Actually, the base fee will be a bit lower next block, since our blocks
-		// are always mostly empty. But the difference will be much less than the
-		// exchange rate of 2, so the test will still check that we take the fee
-		// currency into account everywhere.
-		const maxFeePerGas = block.baseFeePerGas / 2n;
+		const maxFeePerGas = rate.toFeeCurrency(block.baseFeePerGas)+2n;
 		const request = await walletClient.prepareTransactionRequest({
 			account,
 			to: "0x00000000000000000000000000000000DeaDBeef",
 			value: 2,
-			gas: 90000,
-			feeCurrency: process.env.FEE_CURRENCY2,
-			maxFeePerGas,
-			maxPriorityFeePerGas: 1n,
+			gas: 171000,
+			feeCurrency: process.env.FEE_CURRENCY,
+			maxFeePerGas: maxFeePerGas,
+			maxPriorityFeePerGas: 2n,


It seems that the main point of this is that the fee should be too low for native currency, but if the node converts it then it is sufficient. But for USDC it should actually be fine, since the exchange rate results in requiring a lower value for the base-fee compared to when it is denominated in Celo.

ezdac · 2024-10-17T22:28:36Z

e2e_test/js-tests/test_viem_tx.mjs

 			to: "0x00000000000000000000000000000000DeaDBeef",
 			value: 3,
-			gas: 90000,
+			// dynaimcally retrieve intrinsic gas from feeCurrency directory "feeCurrencyConfig"


Do you still plan to implement this for this PR? If not, I would remove the line or add a TODO:, because it doesn't reflect what's happening ;)

Ok removed and I added a ticket for it - #260

e2e_test/debug-fee-currency/lib.sh

e2e_test/js-tests/package.json

e2e_test/run_all_tests.sh

e2e_test/js-tests/test_viem_tx.mjs

karlb

Good feature and a bunch of small improvements on top of that!

e2e_test/js-tests/test_viem_tx.mjs

e2e_test/test_fee_currency_fails_intrinsic.sh

This ensures the jobs get killed however the script exits.

Refactors debug fee currency deployment ensuring fee currency cleanup. The geth-devmode predeployed "oracle3" is reused throughout the e2e tests for all dynamically deployed fee-currencies. However the tested fee-currencies are never removed from the FeeCurrencyDirectory. This causes the node to try to query exchange-rates for the leftover fee-currencies. This fails, because the oracle associated with the old fee-currency has a different token address assigned and thus causes a revert. This issue is not critical and only causes emission of a log message and some unneccessary evm calls, but it is better practice to clean up the fee-currency from the directory anyways. Also adds a test of transactions that exceed the max intrinsic gas for a fee currency, this checks that the transaction fails and the fee currency is blocked. Allows tests to be run on Alfajores. This entailed increasing gas limits for a number of transactions that interact with fee currencies, since the fee currencies on alfajores are different to our debug ones and also tests needed to be updated to not rely on hardcoded fee currency rates. Also adds tests for fee currency use with the gas price RPC API Skips test_fee_currency_fails_on_credit.sh as it seems to be broken.

…ores (#212,#258,#268,#293) Refactors debug fee currency deployment ensuring fee currency cleanup. The geth-devmode predeployed "oracle3" is reused throughout the e2e tests for all dynamically deployed fee-currencies. However the tested fee-currencies are never removed from the FeeCurrencyDirectory. This causes the node to try to query exchange-rates for the leftover fee-currencies. This fails, because the oracle associated with the old fee-currency has a different token address assigned and thus causes a revert. This issue is not critical and only causes emission of a log message and some unneccessary evm calls, but it is better practice to clean up the fee-currency from the directory anyways. Also adds a test of transactions that exceed the max intrinsic gas for a fee currency, this checks that the transaction fails and the fee currency is blocked. Allows tests to be run on Alfajores. This entailed increasing gas limits for a number of transactions that interact with fee currencies, since the fee currencies on alfajores are different to our debug ones and also tests needed to be updated to not rely on hardcoded fee currency rates. Also adds tests for fee currency use with the gas price RPC API Skips test_fee_currency_fails_on_credit.sh as it seems to be broken.

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Adds a test of transactions that exceed the max intrinsic gas for a fee currency, this checks that the transaction fails and the fee currency is blocked. This entailed increasing gas limits for a number of transactions that interact with fee currencies, since the fee currencies on alfajores are different to our debug ones and also tests needed to be updated to not rely on hardcoded fee currency rates. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Refactors debug fee currency deployment ensuring fee currency cleanup. The geth-devmode predeployed "oracle3" is reused throughout the e2e tests for all dynamically deployed fee-currencies. However the tested fee-currencies are never removed from the FeeCurrencyDirectory. This causes the node to try to query exchange-rates for the leftover fee-currencies. This fails, because the oracle associated with the old fee-currency has a different token address assigned and thus causes a revert. This issue is not critical and only causes emission of a log message and some unneccessary evm calls, but it is better practice to clean up the fee-currency from the directory anyways. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Adds a test of transactions that exceed the max intrinsic gas for a fee currency, this checks that the transaction fails and the fee currency is blocked. This entailed increasing gas limits for a number of transactions that interact with fee currencies, since the fee currencies on alfajores are different to our debug ones and also tests needed to be updated to not rely on hardcoded fee currency rates. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Refactors debug fee currency deployment ensuring fee currency cleanup. The geth-devmode predeployed "oracle3" is reused throughout the e2e tests for all dynamically deployed fee-currencies. However the tested fee-currencies are never removed from the FeeCurrencyDirectory. This causes the node to try to query exchange-rates for the leftover fee-currencies. This fails, because the oracle associated with the old fee-currency has a different token address assigned and thus causes a revert. This issue is not critical and only causes emission of a log message and some unneccessary evm calls, but it is better practice to clean up the fee-currency from the directory anyways. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Adds a test of transactions that exceed the max intrinsic gas for a fee currency, this checks that the transaction fails and the fee currency is blocked. This entailed increasing gas limits for a number of transactions that interact with fee currencies, since the fee currencies on alfajores are different to our debug ones and also tests needed to be updated to not rely on hardcoded fee currency rates. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Refactors debug fee currency deployment ensuring fee currency cleanup. The geth-devmode predeployed "oracle3" is reused throughout the e2e tests for all dynamically deployed fee-currencies. However the tested fee-currencies are never removed from the FeeCurrencyDirectory. This causes the node to try to query exchange-rates for the leftover fee-currencies. This fails, because the oracle associated with the old fee-currency has a different token address assigned and thus causes a revert. This issue is not critical and only causes emission of a log message and some unneccessary evm calls, but it is better practice to clean up the fee-currency from the directory anyways. Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

To exercise the fee currency support and test JS lib compatibility. Closes celo-org/optimism#61 e2e(runner): Allows tests to be run on Alfajores. (#212, #258) Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> e2e(fee_currency): Reenable test_fee_currency_fails_on_credit.sh (#400) It has been disabled in d9518cc#diff-7e9dfb8659cce411851abf104e7bf34d826781fb4dc07fc9cb3a1a5b82fa5af1R48-R54, but since it works fine for me now, I don't see a reason to keep it disabled. e2e(runner): Add mainner & baklava e2e test config for easier testing (#331, #340) Run tests against baklava with `NETWORK=baklava ./run_all_tests.sh` after giving the wallet enough balance (or setting your own). e2e(js) update e2e test js dependencies (#380) e2e: Bump braces from 3.0.2 to 3.0.3 in /e2e_test/js-tests (#392) Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-version: 3.0.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

piersy added 8 commits October 16, 2024 21:35

Update viem

09977ea

Improve error handling and logging

0078d6a

Make test fee currency addresses more searchable

d9fa982

Add support for running against alfajores

bc0c531

Update tests to work on alfajores

4dca894

Wait for first failed transaction

c3c4516

As opposed to sending it async, this ensures that we don't end up with an invalid nonce on the next transaction.

Ensure that tail processes are killed

249fde1

Add ci step to run e2e tests on alfajores

f6b13d6

piersy requested a review from karlb October 16, 2024 21:41

Fix just enough test

53d6ce0

piersy commented Oct 17, 2024

View reviewed changes

piersy added 5 commits October 17, 2024 12:45

Add debug

7a01396

Re-instate previous log capture kill approach

920c9d8

Fix script dir location

ed872ee

Ensure that send_tx script exits once done

dcb637a

Without this it can hang for about 12 minutes!

Ensure that we match system addresses irrespective of casing

910a033

ezdac reviewed Oct 17, 2024

View reviewed changes

karlb reviewed Oct 18, 2024

View reviewed changes

e2e_test/debug-fee-currency/lib.sh Outdated Show resolved Hide resolved

e2e_test/js-tests/package.json Show resolved Hide resolved

e2e_test/run_all_tests.sh Show resolved Hide resolved

karlb reviewed Oct 18, 2024

View reviewed changes

e2e_test/js-tests/test_viem_tx.mjs Outdated Show resolved Hide resolved

karlb reviewed Oct 18, 2024

View reviewed changes

e2e_test/js-tests/test_viem_tx.mjs Outdated Show resolved Hide resolved

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

e2e_test/test_fee_currency_fails_intrinsic.sh Outdated Show resolved Hide resolved

piersy added 7 commits October 18, 2024 17:35

Fix typo

1ade24c

Add comment to just high enough test

6c3343b

Ensure we get gas fees in fee currency

2a9d6fe

Remove stray comment

ec2b25c

Remove un-needed bash option setting

6ae143c

Correct indentation

b819d83

Kill bg jobs with trap

9d54552

This ensures the jobs get killed however the script exits.

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

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

[e2e] Refactor and add e2e tests, run on alfajores (#212,#258,#268,#293)

345006f

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

e2e(runner): Allows tests to be run on Alfajores. (#212, #258)

eb0c5f7

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

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

e2e(runner): Allows tests to be run on Alfajores. (#212, #258)

e6c6619

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

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

e2e(runner): Allows tests to be run on Alfajores. (#212, #258)

06615f8

Co-authored-by: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com>

Conversation

piersy commented Oct 16, 2024

Uh oh!

socket-security bot commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

piersy Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piersy Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

ezdac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezdac Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

piersy Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

karlb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

socket-security bot commented Oct 16, 2024 •

edited

Loading

ezdac Oct 17, 2024 •

edited

Loading

ezdac Oct 17, 2024 •

edited

Loading