Skip to content

[Not to be merged] Integrate Simplex within Avalanchego #3957

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samliok
Copy link
Contributor

@samliok samliok commented May 12, 2025

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

@Copilot Copilot AI review requested due to automatic review settings May 12, 2025 17:23
@samliok samliok marked this pull request as draft May 12, 2025 17:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@yacovm yacovm changed the title Integrate Simplex within Avalanchego [Not to be merged] Integrate Simplex within Avalanchego May 12, 2025
@@ -24,7 +24,7 @@ const (
DefaultNetworkTimeout = 2 * time.Minute

// Minimum required to ensure connectivity-based health checks will pass
DefaultNodeCount = 2
DefaultNodeCount = 4
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Note that this can also be configured with the --node-count flag.

@@ -57,7 +57,7 @@ func DefaultTmpnetFlags() FlagsMap {
config.PublicIPKey: "127.0.0.1",
config.HTTPHostKey: "127.0.0.1",
config.StakingHostKey: "127.0.0.1",
config.LogDisplayLevelKey: logging.Off.String(), // Display logging not needed since nodes run headless
config.LogDisplayLevelKey: logging.Info.String(), // Display logging not needed since nodes run headless
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Note that master no longer sets a value for log display level, so the value ends up the same as log-level.

Comment on lines +1489 to +1500
if m.TracingEnabled {
messageSender = sender.Trace(messageSender, m.Tracer)
}

chainConfig, err := m.getChainConfig(ctx.ChainID)
if err != nil {
return nil, fmt.Errorf("error while fetching chain config: %w", err)
}

if m.TracingEnabled {
vm = tracedvm.NewBlockVM(vm, primaryAlias, m.Tracer)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yacovm what is the purpose of setting the vm twice if tracing is enabled?

	vm = tracedvm.NewBlockVM(vm, primaryAlias, m.Tracer)

Copy link
Contributor

Choose a reason for hiding this comment

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

it just wraps the VM with a tracing VM

Copy link

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will cause this PR to ignore lifecycle events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants