Add Isthmus ERC-20 bridge devnet-sdk test by teddyknox · Pull Request #14631 · ethereum-optimism/optimism

teddyknox · 2025-03-04T20:29:11Z

This PR ports a few tests from the op-e2e test framework to the devnet-sdk framework, and makes a few necessary changes to the devnet-sdk framework. Since devnet-sdk tests are not run as part of CI yet, you can run tests with the following procedure.

Running devnet-sdk tests in kurtosis-devnet/tests

cd kurtosis-devnet
just isthmus-devnet
DEVNET_ENV_URL=kt://isthmus-devnet/devnet go test -v ./tests/...

You will see certain tests get skipped because they target a different fork than simple-devnet runs (Holocene). Feel free to try with different devnets.

Change Summary

New Test Files Added:
- tests/fees/fees_test.go: Tests for L1 and L2 fee calculations across different fork configurations (pre-regolith, regolith, ecotone, fjord)
- tests/fjord/check_scripts_test.go: Tests for the check-fjord script functionality
- tests/isthmus/erc20_bridge_test.go: Tests for ERC20 token bridging between L1 and L2 chains
- tests/isthmus/withdrawal_root_test.go: Tests that the withdrawal root is properly set on the block header as a withdrawal is performed.
Core Framework Enhancements:
- Added new chain interface methods in devnet-sdk/system/interfaces.go:
  - ChainConfig()
  - BlockByNumber()
  - LatestBlock()
  - BlockByHash()
  - Addresses()
- Enhanced Artifact struct in devnet-sdk/kt/fs/fs.go with improved file extraction capabilities (allows multiple reads from a single artifact object).
- Added new fork validator configuration management in devnet-sdk/testing/testlib/validators/forks.go
Testing Infrastructure Improvements:
- Added new validators for fork configurations and system validation
- Enhanced wallet management with separate L1 and L2 wallet handling
- Improved genesis file handling and chain configuration management
Documentation Updates:
- Added detailed flowcharts in std_output.md for devnet descriptor generation
- Enhanced documentation for wallet validation and chain configuration
Chain Configuration Updates:
- Added support for new fork configurations including Isthmus and Jovian
- Enhanced chain parameter handling with new fields like IsthmusTimeOffset

The changes maintain backward compatibility while adding new testing capabilities that will help ensure the reliability of the Optimism network across different configurations and features.

teddyknox · 2025-03-04T20:32:13Z

/ci authorize 061ab2f

This commit also makes changes to devnet-sdk and kurtosis-devnet that enable the new tests.

codecov · 2025-03-04T21:07:00Z

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 45.46%. Comparing base (aa132b3) to head (577c9aa).
Report is 85 commits behind head on develop.

Files with missing lines	Patch %	Lines
op-service/sources/eth_client.go	0.00%	15 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #14631      +/-   ##
===========================================
- Coverage    46.73%   45.46%   -1.27%     
===========================================
  Files         1040      982      -58     
  Lines        89406    84719    -4687     
===========================================
- Hits         41785    38521    -3264     
+ Misses       44533    43285    -1248     
+ Partials      3088     2913     -175

Flag	Coverage Δ
cannon-go-tests-32	`?`
cannon-go-tests-64	`?`
contracts-bedrock-tests	`94.48% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
op-node/rollup/chain_spec.go	`69.64% <ø> (ø)`
op-service/sources/eth_client.go	`26.05% <0.00%> (-1.59%)`	⬇️

... and 65 files with indirect coverage changes

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sigma · 2025-03-05T21:19:04Z

devnet-sdk/system/interfaces.go

 	GasPrice(ctx context.Context) (*big.Int, error)
 	GasLimit(ctx context.Context, tx TransactionData) (uint64, error)
 	PendingNonceAt(ctx context.Context, address common.Address) (uint64, error)
+	ChainConfig() (*params.ChainConfig, error)


genuine question: should the ChainConfig be an interface i nstead of params.ChainConfig?
same for the Block type below.

I guess I'm a bit worried about the feasibility of providing alternative implementations of these concepts if we leak the concrete types here.

Ideally it would be possible to have access to the op-geth chain config in the event that we're testing op-geth specifically, and access to a higher-level interface if we don't care which execution client we're working with. I'd be happy to add the interface for now.

pcw109550 · 2025-03-06T07:06:50Z

kurtosis-devnet/tests/fees/fees_test.go

+)
+
+func init() {
+	slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug})))


Need a rebase for removing slog usage. Please refer #14644

pcw109550

Is #14668 intended to separate this PR to smaller chunks? If then, will review after building block PR is merged

pcw109550 · 2025-03-06T14:16:52Z

devnet-sdk/system/chain.go

+	clients     *clientManager
+	registry    interfaces.ContractsRegistry
+	mu          sync.Mutex
+	chainConfig *params.ChainConfig


Feels like we need a unification here. #14668 adds descriptors.ChainConfig.

pcw109550 · 2025-03-06T14:19:40Z

devnet-sdk/system/system.go

 			if err != nil {
 				return fmt.Errorf("failed to add L1 chain: %w", err)
 			}
+			fmt.Println("L1 chain ID: ", l1.ID())


Maybe this print should be removed?

pcw109550 · 2025-03-06T14:20:20Z

devnet-sdk/system/system.go

 func (s *system) addChains(chains ...*descriptors.Chain) error {
 	for _, chainDesc := range chains {
-		if chainDesc.ID == "" {
+		if chainDesc.Name == "Ethereum" {


ditto: #14668 (comment)

teddyknox · 2025-03-06T23:00:26Z

Closing in favor of smaller PRs such as #14668

This was referenced Mar 4, 2025

DRAFT: Update withdrawals root devnet-sdk test #14505

Closed

DRAFT: Add a few new kurtosis-devnet tests #14567

Closed

teddyknox marked this pull request as ready for review March 4, 2025 20:32

teddyknox requested review from a team as code owners March 4, 2025 20:32

teddyknox requested a review from ajsutton March 4, 2025 20:32

teddyknox changed the title ~~Add a few new kurtosis-devnet tests~~ Add Isthmus ERC-20 bridge devnet-sdk test Mar 4, 2025

teddyknox requested review from janjakubnanista and sigma March 4, 2025 20:33

Add a few new kurtosis-devnet tests

31f4e0c

This commit also makes changes to devnet-sdk and kurtosis-devnet that enable the new tests.

teddyknox force-pushed the devnet-sdk-test-porting-1 branch from 061ab2f to 31f4e0c Compare March 4, 2025 20:57

Update mockChain definition to match Chain interface

f56afba

teddyknox added 4 commits March 4, 2025 16:21

Fix lint errors

9772f1e

Fix context key sentinel values to byte pointers

a80d5d9

Fix another lint error

02e73af

Add Isthmus withdrawal root test

577c9aa

teddyknox requested a review from pcw109550 March 5, 2025 03:48

sigma reviewed Mar 5, 2025

View reviewed changes

pcw109550 mentioned this pull request Mar 6, 2025

devnet-sdk: Take out RPC functionality as Node interface from Chain Interface #14660

Merged

pcw109550 reviewed Mar 6, 2025

View reviewed changes

teddyknox mentioned this pull request Mar 6, 2025

Add devnet-sdk Fork Configuration Validators #14668

Merged

teddyknox closed this Mar 6, 2025

teddyknox added the A-kt-devnet Area: kurtosis devnet label Mar 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Isthmus ERC-20 bridge devnet-sdk test#14631

Add Isthmus ERC-20 bridge devnet-sdk test#14631
teddyknox wants to merge 6 commits intodevelopfrom
devnet-sdk-test-porting-1

teddyknox commented Mar 4, 2025 •

edited

Loading

Uh oh!

teddyknox commented Mar 4, 2025

Uh oh!

codecov bot commented Mar 4, 2025 •

edited

Loading

Uh oh!

sigma Mar 5, 2025

Uh oh!

teddyknox Mar 6, 2025

Uh oh!

pcw109550 Mar 6, 2025

Uh oh!

pcw109550 left a comment

Uh oh!

pcw109550 Mar 6, 2025

Uh oh!

pcw109550 Mar 6, 2025

Uh oh!

pcw109550 Mar 6, 2025

Uh oh!

teddyknox commented Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

teddyknox commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Running devnet-sdk tests in kurtosis-devnet/tests

Change Summary

Uh oh!

teddyknox commented Mar 4, 2025

Uh oh!

codecov bot commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sigma Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

teddyknox Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

pcw109550 Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

pcw109550 left a comment

Choose a reason for hiding this comment

Uh oh!

pcw109550 Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

pcw109550 Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

pcw109550 Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

teddyknox commented Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

teddyknox commented Mar 4, 2025 •

edited

Loading

codecov bot commented Mar 4, 2025 •

edited

Loading