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

[Execution] Shadow Execution node implementation #5411

Merged
merged 51 commits into from
Mar 1, 2024

Conversation

zhangchiqing
Copy link
Member

@zhangchiqing zhangchiqing commented Feb 16, 2024

Close #5118

Adding observer mode to execution node so that a unstaked EN can sync blocks and collections from a staked public access node and execute blocks. It's useful for testing execution node changes, as it can run and execute on its own with new changes.

How to test:

# start the localnet with test execution node running 
cd integration/localnet
make stop; make bootstrap-test-en; make start;

# sending transaction to validate the execution
cd integration/benchmark/cmd/manual
go run ./main.go

# expect to see `block executed` log from the test execution node
docker logs -f localnet-test_execution_1-1

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2024

Codecov Report

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

Project coverage is 59.90%. Comparing base (1726b7b) to head (f265dc9).

Files Patch % Lines
engine/execution/provider/engine.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5411      +/-   ##
==========================================
+ Coverage   55.97%   59.90%   +3.93%     
==========================================
  Files        1022      543     -479     
  Lines       99705    51998   -47707     
==========================================
- Hits        55807    31150   -24657     
+ Misses      39598    18532   -21066     
+ Partials     4300     2316    -1984     
Flag Coverage Δ
unittests 59.90% <0.00%> (+3.93%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhangchiqing zhangchiqing force-pushed the leo/localnet-test-en branch 2 times, most recently from f42a417 to c4ac5bd Compare February 22, 2024 01:40
@zhangchiqing zhangchiqing marked this pull request as ready for review February 22, 2024 01:40
network/noop.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this to network/underlay sub-package.

error,
) {
if !node.ObserverMode {
return &module.NoopReadyDoneAware{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a log indicating whether or not this module was run

Comment on lines 968 to 970
// Execution Data cache that uses a blobstore as the backend (instead of a downloader)
// This ensures that it simply returns a not found error if the blob doesn't exist
// instead of attempting to download it from the network.
Copy link
Contributor

Choose a reason for hiding this comment

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

this comment isn't right. it's actually using a downloader (correctly).

Copy link
Member Author

Choose a reason for hiding this comment

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

exeNode.followerDistributor.AddOnBlockFinalizedConsumer(r.OnBlockFinalized)

execDataDistributor.AddOnExecutionDataReceivedConsumer(func(data *execution_data.BlockExecutionDataEntity) {
if len(data.BlockExecutionData.ChunkExecutionDatas) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

did you find that this case was encountered? There should always be at least one chunk because of the system chunk

cmd/utils.go Outdated
) (
p2p.IDTranslator,
func() module.IdentifierProvider,
error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
error) {
error,
) {

@zhangchiqing zhangchiqing force-pushed the leo/localnet-test-en branch 3 times, most recently from 5e08a8f to 023479f Compare February 28, 2024 20:47
@zhangchiqing zhangchiqing enabled auto-merge March 1, 2024 00:48
@@ -97,13 +103,13 @@ func WriteFile(path string, data []byte) error {
return err
}

func WriteObserverPrivateKey(observerName, bootstrapDir string) error {
func WriteObserverPrivateKey(observerName, bootstrapDir string) (crypto.PrivateKey, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this new return is never used.

@zhangchiqing zhangchiqing added this pull request to the merge queue Mar 1, 2024
Merged via the queue into master with commit 057d89b Mar 1, 2024
51 checks passed
@zhangchiqing zhangchiqing deleted the leo/localnet-test-en branch March 1, 2024 15:50
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.

Creating test environment for execution node that can follow and execute latest blocks
5 participants