Skip to content

isthmus: withdrawals-root test in NAT#14353

Closed
protolambda wants to merge 2 commits intodevelopfrom
withdrawals-root
Closed

isthmus: withdrawals-root test in NAT#14353
protolambda wants to merge 2 commits intodevelopfrom
withdrawals-root

Conversation

@protolambda
Copy link
Contributor

@protolambda protolambda commented Feb 13, 2025

Description

Example isthmus withdrawals-root NAT test, on initial devnet-SDK.

Note: before running this, don't forget to configure Kurtosis with the Isthmus fork

Tests

Additional context

Metadata

@protolambda protolambda requested review from emhane and sigma February 13, 2025 17:09
@codecov
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

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

Project coverage is 41.51%. Comparing base (3aa46be) to head (e055165).
Report is 159 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   #14353      +/-   ##
===========================================
- Coverage    45.89%   41.51%   -4.39%     
===========================================
  Files         1007      842     -165     
  Lines        86310    77012    -9298     
===========================================
- Hits         39616    31970    -7646     
+ Misses       43701    42227    -1474     
+ Partials      2993     2815     -178     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests ?

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

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

... and 175 files with indirect coverage changes

@tynes
Copy link
Contributor

tynes commented Feb 13, 2025

consider this library for easy ABI encoding: https://github.com/lmittmann/w3?tab=readme-ov-file#abi-bindings

@protolambda
Copy link
Contributor Author

See #14361 for needed op-node fixes for isthmus to actually run in op-node

See #14356 for kurtosis isthmus example. But instead of configuring it in interop.yaml we instead should have a new isthmus.yaml config in kurtosis devnet directory.

@emhane emhane added H-isthmus Hardfork: change is planned for isthmus upgrade A-devnet-sdk Area: devnet-sdk labels Feb 14, 2025
Copy link
Contributor

@emhane emhane left a comment

Choose a reason for hiding this comment

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

awesome, thanks a lot

priv, err := crypto.HexToECDSA(user.PrivateKey())
require.NoError(t, err)

// construct call input, ugly but no bindings...
Copy link
Contributor

Choose a reason for hiding this comment

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

should we open an issue for writing these bindings? @sigma

Copy link
Contributor

Choose a reason for hiding this comment

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

I can take this #14473

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend just using w3 and not doing anything too fancy here

require.NoError(t, solabi.WriteUint256(&input, big.NewInt(4+20+32+32))) // calldata offset to length data
require.NoError(t, solabi.WriteUint256(&input, big.NewInt(0))) // length

// sign a tx to trigger a withdrawal (no ETH value, just a message), submit it
Copy link
Contributor

Choose a reason for hiding this comment

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

would like to run this part many times in a loop. is best way to re-run test or can make use of pre-funded wallet more than once here by looping some given number of times (number passed as parameter) ?

@teddyknox
Copy link
Contributor

FYI my fixes after rebasing here: #14505

@protolambda
Copy link
Contributor Author

Closing in favor of #14505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-devnet-sdk Area: devnet-sdk H-isthmus Hardfork: change is planned for isthmus upgrade

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments