Run e2e tests on alfajores #258
Conversation
As opposed to sending it async, this ensures that we don't end up with an invalid nonce on the next transaction.
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
e2e_test/js-tests/test_viem_tx.mjs
Outdated
| 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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
e2e_test/js-tests/test_viem_tx.mjs
Outdated
| 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, |
There was a problem hiding this comment.
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.
e2e_test/js-tests/test_viem_tx.mjs
Outdated
| @@ -63,7 +79,8 @@ const testNonceBump = async ( | |||
| account, | |||
| to: "0x00000000000000000000000000000000DeaDBeef", | |||
| value: 3, | |||
| gas: 90000, | |||
| // dynaimcally retrieve intrinsic gas from feeCurrency directory "feeCurrencyConfig" | |||
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
Ok removed and I added a ticket for it - #260
This ensures the jobs get killed however the script exits.
Not that this test is running on live networks we need to account for the case where the base fee could increase, we do this by adding a 10% buffer. Also ensures that we use FEE_CURRENCY2 when running local tests. And finally adds a check to fail the test if the value of the fee currency is not sufficiently greater than the value of celo.
Modifies our e2e tests to be able to run on alfajores if the environment variable
NETWORKis 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.