fix: update fallback transport by spypsy · Pull Request #12470 · AztecProtocol/aztec-packages

spypsy · 2025-03-05T12:06:28Z

fallback transport by viem works by trying the first node it was given, and if it receives an error, it moves on to the next, unless shouldThrow returns true.
shouldThrow however was not triggered on contract errors so with checks like canProposeAt, it would try secondary nodes, even though the 1st node returned the correct result (a contract error)

This implementation of fallback transport throws immediately when we receive any contract error

spalladino

Looks good! Just threw in a few comments. Also sent a quick PR to viem to see if they can add shouldThrow as a config.

spalladino · 2025-03-05T15:01:51Z

yarn-project/archiver/src/archiver/archiver.ts

 import { EventEmitter } from 'events';
 import groupBy from 'lodash.groupby';
-import { type GetContractReturnType, createPublicClient, fallback, getContract, http } from 'viem';
+import { type GetContractReturnType, createPublicClient, getContract, http } from 'viem';


Let's add a rule to eslint so we don't accidentally import viem's fallback instead of our own: https://eslint.org/docs/latest/rules/no-restricted-imports#importnames

spalladino · 2025-03-05T15:06:14Z

yarn-project/ethereum/src/test/fallback_transport.test.ts

+import type { ExtendedViemWalletClient } from '../types.js';
+import { startAnvil } from './start_anvil.js';
+
+describe('fallback_transport', () => {


Let's add a test for the transport not falling back on a contract error (main reason why you implemented this as far as I understand!)

spalladino · 2025-03-07T11:53:14Z

yarn-project/ethereum/src/test/fallback_transport.test.ts

+    // this should revert
+    try {
+      await client.sendTransaction({
+        to: contractAddress,
+        data: '0xcafe',
+      });
+
+      fail('Transaction should have reverted');
+    } catch (error: any) {
+      // The error should be from the first node
+      expect(error.message).toContain('revert');
+
+      // Wait a sec to ensure any potential transaction would have been processed
+      await sleep(1000);
+
+      // Check that no transactions were sent to the contract on the second node
+      const block1 = await publicClient1.getBlock();
+      const block2 = await publicClient2.getBlock();
+
+      expect(block1.transactions.length).toBe(1);
+      expect(block2.transactions.length).toBe(0);
+    }


How does this work? We're sending the tx but not awaiting its receipt. Is it possible the revert we're getting back corresponds to a simulation (or gas estimation), instead of the actual tx? And the block1.transactions.length corresponds to the contract deployment?

yarn-project/ethereum/src/test/fallback_transport.test.ts

spalladino · 2025-03-07T11:54:33Z

FYI viem 2.23.7 was released with the option to customize shouldThrow in the fallback transport, so if upgrading doesn't break things, we can use that to remove the vendored code.

spypsy · 2025-03-07T13:01:40Z

thanks @spalladino, it seems they also changed the default behaviour to what we wanted (not falling back on reverts).
I've kept (& updated) the tests anyway

spalladino · 2025-03-10T12:36:55Z

playground/vite.config.ts

      //   ],
      // }),
      bundlesize({
-        limits: [{ name: "assets/index-*", limit: "1900kB" }],


Shame on you, new viem

spalladino · 2025-03-10T12:40:47Z

yarn-project/yarn.lock

-"viem@npm:2.22.8":
-  version: 2.22.8
-  resolution: "viem@npm:2.22.8"
+"viem@npm:2.23.7":


Heads up we also got a viem 2.23.5 coming from somewhere according to this yarn.lock (see below this viem entry)

woah, good catch! fixing

🤖 I have created a new Aztec Packages release --- ## [0.79.0](v0.78.1...v0.79.0) (2025-03-11) ### ⚠ BREAKING CHANGES * aggregate data for batch calls ([#12562](#12562)) ### Features * add extra attributes to target_info ([#12583](#12583)) ([c296422](c296422)) * add optional oracle resolver url in `acvm_cli` (noir-lang/noir#7630) ([cc6cdbb](cc6cdbb)) * allow to pay via sponsored fpc from cli ([#12598](#12598)) ([877de5c](877de5c)) * array concat method (noir-lang/noir#7199) ([cc6cdbb](cc6cdbb)) * **avm:** ToRadix gadget ([#12528](#12528)) ([02a7171](02a7171)) * aztec-up -v flag ([#12590](#12590)) ([6a41565](6a41565)) * **bb:** consider polynomial end_index when constructing partially evaluated multivariates ([#12530](#12530)) ([abd22cd](abd22cd)) * **config:** add fallbacks ([#12593](#12593)) ([f2f9ef3](f2f9ef3)) * **p2p:** add trusted peers mechanics ([#12447](#12447)) ([d67f7e8](d67f7e8)) * **p2p:** peer manager peer count metrics ([#12575](#12575)) ([b4891c1](b4891c1)) * provision alerts ([#12561](#12561)) ([2ea1767](2ea1767)) * Resolve callstacks in protocol circuit errors on wasm ([#12573](#12573)) ([657299b](657299b)) ### Bug Fixes * aggregate data for batch calls ([#12562](#12562)) ([bd0b3b6](bd0b3b6)) * broken kind transfer test ([#12611](#12611)) ([6e91934](6e91934)) * Cl/release fixes 2 ([#12595](#12595)) ([fc597f4](fc597f4)) * Cl/release noir refs ([#12597](#12597)) ([fdcfcaf](fdcfcaf)) * demote log ([#12626](#12626)) ([bec8953](bec8953)) * deploy method test ([#12609](#12609)) ([f2c06c2](f2c06c2)) * Do not report epoch as complete until blocks have synced ([#12638](#12638)) ([2ddfa76](2ddfa76)), closes [#12625](#12625) * Error on infinitely recursive types (noir-lang/noir#7579) ([cc6cdbb](cc6cdbb)) * get L1 tx utils config from env ([#12620](#12620)) ([d930c01](d930c01)) * Log overflow handling in reset ([#12579](#12579)) ([283b624](283b624)) * metrics update ([#12571](#12571)) ([80a5df2](80a5df2)) * **sandbox:** query release please manifest for version if in a docker container ([#12591](#12591)) ([db8ebc6](db8ebc6)) * **spartan:** setup needs kubectl ([#12580](#12580)) ([753cb33](753cb33)) * update dead partial notes link ([#12629](#12629)) ([5a1dc4c](5a1dc4c)) * update error message to display 128 bits as valid bit size (noir-lang/noir#7626) ([cc6cdbb](cc6cdbb)) * update fallback transport ([#12470](#12470)) ([88f0711](88f0711)) ### Miscellaneous * bump external pinned commits (noir-lang/noir#7640) ([cc6cdbb](cc6cdbb)) * **ci3:** add helper for uncached test introspection ([#12618](#12618)) ([9ac518b](9ac518b)) * **ci3:** better memsuspend_limit comment ([#12622](#12622)) ([de84187](de84187)) * clean up upgrade test and other small things ([#12558](#12558)) ([c28abe1](c28abe1)) * cleanup eth artifacts + misc aztec.js reorg ([#12563](#12563)) ([6623244](6623244)) * **docs:** Updated accounts page ([#12019](#12019)) ([d45dac9](d45dac9)) * Fix mac build ([#12610](#12610)) ([adceed6](adceed6)) * gemini soundness regression test ([#12570](#12570)) ([c654106](c654106)) * more sane e2e_prover/full timeout ([#12619](#12619)) ([add9d35](add9d35)) * reactivate acir_test for `regression_5045` ([#12548](#12548)) ([c89f89c](c89f89c)) * remove unnecessary trait bounds (noir-lang/noir#7635) ([cc6cdbb](cc6cdbb)) * Rename `StructDefinition` to `TypeDefinition` (noir-lang/noir#7614) ([cc6cdbb](cc6cdbb)) * replace relative paths to noir-protocol-circuits ([4f7f5c3](4f7f5c3)) * replace relative paths to noir-protocol-circuits ([0f68d11](0f68d11)) * replace relative paths to noir-protocol-circuits ([8f593ce](8f593ce)) * replace relative paths to noir-protocol-circuits ([251ae38](251ae38)) * rollup library cleanup ([#12621](#12621)) ([361fc59](361fc59)) * **sandbox:** drop cheat-codes log level ([#12586](#12586)) ([24f04c7](24f04c7)) * **sandbox:** expose anvil port ([#12599](#12599)) ([955f1b0](955f1b0)) * **testnet:** updating script for ignition, change naming ([#12566](#12566)) ([2d7b69d](2d7b69d)) * turn on masking in eccvm ([#12467](#12467)) ([aacb91a](aacb91a)) * Update Bb line counting script ([#12350](#12350)) ([7a41843](7a41843)) * update docs to reflect u128 type (noir-lang/noir#7623) ([cc6cdbb](cc6cdbb)) * Validate blobs posted to sink belong to our L2 ([#12587](#12587)) ([9578f1e](9578f1e)), closes [#12497](#12497) ### Documentation * update cli-wallet commands in profiler doc ([#12568](#12568)) ([239a4fb](239a4fb)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

spypsy added 2 commits March 5, 2025 12:03

feat: custom viem fallback transport

f2f3f91

Merge branch 'master' into spy/fallback-transport

38a3c66

spypsy marked this pull request as ready for review March 5, 2025 12:06

add @aztec/ethereum dep

077b1d3

spypsy added the ci-full Run all master checks. label Mar 5, 2025

spypsy and others added 3 commits March 5, 2025 13:22

add fallback transport test

01f5af4

rm unused

02aa26b

Merge branch 'master' into spy/fallback-transport

ed09399

spalladino reviewed Mar 5, 2025

View reviewed changes

PR fixes

954db91

spalladino reviewed Mar 7, 2025

View reviewed changes

spypsy changed the title ~~fix: custom fallback transport~~ fix: update fallback transport Mar 7, 2025

spypsy added 2 commits March 7, 2025 13:00

update viem, remove custom fallback

61abdce

Merge branch 'master' into spy/fallback-transport

d7cc79d

spypsy added 6 commits March 7, 2025 13:03

update cli

26c837d

remove aztec/ethereum from blob-sink

bfcc0ac

update viem in cli

c656d6f

update yarn.lock

f8cb869

rm async

64e1d0e

increase playground bundle size

6128256

spalladino reviewed Mar 10, 2025

View reviewed changes

spypsy added 4 commits March 10, 2025 13:25

update viem in stdlib

0ce2893

Merge branch 'master' into spy/fallback-transport

84fb7f4

fix yarn.lock

f3d7a2b

update yarn.lock again

98a4fdf

spalladino approved these changes Mar 10, 2025

View reviewed changes

spypsy merged commit 88f0711 into master Mar 10, 2025
6 checks passed

spypsy deleted the spy/fallback-transport branch March 10, 2025 17:13

AztecBot mentioned this pull request Mar 10, 2025

chore(master): release 0.79.0 #12578

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update fallback transport#12470

fix: update fallback transport#12470
spypsy merged 19 commits intomasterfrom
spy/fallback-transport

spypsy commented Mar 5, 2025

Uh oh!

spalladino left a comment

Uh oh!

spalladino Mar 5, 2025

Uh oh!

spalladino Mar 5, 2025

Uh oh!

spypsy Mar 6, 2025

Uh oh!

spalladino Mar 7, 2025

Uh oh!

Uh oh!

spalladino commented Mar 7, 2025

Uh oh!

spypsy commented Mar 7, 2025

Uh oh!

spalladino Mar 10, 2025

Uh oh!

spalladino Mar 10, 2025

Uh oh!

spypsy Mar 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

spypsy commented Mar 5, 2025

Uh oh!

spalladino left a comment

Choose a reason for hiding this comment

Uh oh!

spalladino Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

spalladino Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

spypsy Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

spalladino Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spalladino commented Mar 7, 2025

Uh oh!

spypsy commented Mar 7, 2025

Uh oh!

spalladino Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

spalladino Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

spypsy Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants