Skip to content

chore: Fix and reenable fees-settings test #12302

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

Merged
merged 1 commit into from
Feb 26, 2025
Merged

Conversation

spalladino
Copy link
Contributor

Fixes #12258

@spalladino spalladino removed the request for review from charlielye February 26, 2025 18:36
@spalladino spalladino enabled auto-merge (squash) February 26, 2025 19:26
@@ -47,7 +47,10 @@ export class GasTxValidator implements TxValidator<Tx> {
maxFeesPerGas.feePerL2Gas.lt(this.#gasFees.feePerL2Gas);

if (notEnoughMaxFees) {
this.#log.warn(`Skipping transaction ${tx.getTxHash()} due to insufficient fee per gas`);
this.#log.warn(`Skipping transaction ${await tx.getTxHash()} due to insufficient fee per gas`, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose the alternative to this making the whole func async would be to kick this off as an async side-effect. But perhaps would get weird

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The caller was already async, so I didn't worry too much

@spalladino spalladino merged commit dbcb2b1 into master Feb 26, 2025
6 checks passed
@spalladino spalladino deleted the palla/fee-settings branch February 26, 2025 21:45
spalladino added a commit that referenced this pull request Feb 26, 2025
TomAFrench added a commit that referenced this pull request Feb 26, 2025
* master: (31 commits)
  feat: Slack message to ci channel tagging owners on flakes. (#12284)
  fix: slack notify was broken by quoted commit titles
  revert: "chore: Fix and reenable fees-settings test (#12302)"
  fix: run arm64 on master (#12307)
  yolo fix
  chore: Fix and reenable fees-settings test (#12302)
  feat!: rename compute_nullifier_without_context (#12308)
  chore: Lazy loading artifacts everywhere (#12285)
  chore: Reenable dapp subscription test (#12304)
  chore: Run prover test with fake proofs when requested (#12305)
  chore: Do not set CI_FULL outside CI (#12300)
  chore: new mnemonic deployments on sepolia (#12076)
  chore!: enable multiple L1 nodes to be used (#11945)
  chore: remove no longer supported extension from vscode/extension.json (#12303)
  fix(e2e): p2p_reqresp (#12297)
  feat: Sync from noir (#12298)
  chore: enabling `e2e_contract_updates` in CI + nuking irrelevant test (#12293)
  feat: prepend based merge (#12093)
  feat: fetch addresses from registry (#12000)
  feat: live logs (#12271)
  ...
PhilWindle pushed a commit that referenced this pull request Feb 28, 2025
Second iteration of
#12302. This passed
a local grind with:
```
seq 1 10 | parallel --bar --halt now,fail=1 "yarn-project/end-to-end/scripts/run_test.sh simple e2e_fees/fee_settings >/dev/null"
```

Fixes #12258
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.

Fix fee_settings.test.ts and add it to CI
2 participants