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

Abstract block info #105

Merged
merged 8 commits into from
Sep 3, 2024
Merged

Abstract block info #105

merged 8 commits into from
Sep 3, 2024

Conversation

syntrust
Copy link
Contributor

@syntrust syntrust commented Aug 20, 2024

When mining on L2 sometimes run into "StorageContract: minedTs too small".

This could be an edge case when a blob in a new shard happens to be mined immediately after the shard is opened by pubBlobs.

The test can be done by running the following cmd with code ethstorage/es-node#187

env ES_NODE_SIGNER_PRIVATE_KEY=<pk> ./integration_tests/run_tests.sh

The reason is that miningInfo is timestamped by L2 chain which has block time of 2 seconds while mine method computes minedTs with L1 blocks of 12 seconds block time.

This PR abstracts blockNumber() and blockTs() as well as getRandao, etc. for StorageContract and its L2 extender so that they can use different values and implementations.

@syntrust syntrust changed the base branch from main to fix_xshard August 20, 2024 09:02
@syntrust syntrust changed the base branch from fix_xshard to main August 20, 2024 09:03
@syntrust syntrust changed the base branch from main to fix_xshard August 20, 2024 09:48
@syntrust syntrust requested a review from qzhodl August 20, 2024 10:10
contracts/EthStorageContractL2.sol Outdated Show resolved Hide resolved
contracts/test/TestEthStorageContract.sol Outdated Show resolved Hide resolved
@syntrust syntrust requested a review from qzhodl August 29, 2024 03:38
@syntrust syntrust merged commit 3821e3c into fix_xshard Sep 3, 2024
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.

3 participants