Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ACP-176 e2e tests #3749

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft

Add ACP-176 e2e tests #3749

wants to merge 17 commits into from

Conversation

StephenButtolph
Copy link
Contributor

@StephenButtolph StephenButtolph commented Feb 27, 2025

Why this should be merged

This PR:

  • Adds a post_fortuna e2e CI run.
  • Modifies the gas consumption in the dynamic fees test to explicitly specify the amount of gas consumed in the tx (rather than using a value specified by the solidity file)
  • Removes the usage of legacy transactions - dynamic fee txs should always be preferred
  • Removes the usage of the SuggestGasPrice as the price APIs do not seem reliable. We should consider changing how the price APIs work from inspecting prior blocks to just returning the next baseFee.

How this works

Just modifying tests.

How this was tested

CI

Need to be documented in RELEASES.md?

No.

Comment on lines -4 to -5
// AUTOMATICALLY GENERATED. DO NOT EDIT!
// Generated from hashing.sol by compile-contract.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file doesn't seem to have ever been checked in to the repo (even going back to when this file was originally added to the kurtosis e2e tests).

@@ -183,7 +183,12 @@ func SendEthTransaction(tc tests.TestContext, ethClient ethclient.Client, signed
return true
}, DefaultTimeout, DefaultPollingInterval, "failed to see transaction acceptance before timeout")

require.Equal(types.ReceiptStatusSuccessful, receipt.Status)
Copy link
Contributor Author

@StephenButtolph StephenButtolph Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implied that we would only want to issue successful transactions. The gas consumption in this PR actually expects the transaction to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant