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

WIP host tracking #136

Merged
merged 4 commits into from
Apr 25, 2024
Merged

WIP host tracking #136

merged 4 commits into from
Apr 25, 2024

Conversation

N0xMare
Copy link
Contributor

@N0xMare N0xMare commented Apr 24, 2024

meant to address issue #132 Host Tracking

Copy link
Collaborator

@clabby clabby left a comment

Choose a reason for hiding this comment

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

Looking good so far.

To get this in, we'll need to make some edits to CI that ensures that the host is not built or linted with the cannon/asterisc build pipelines, as it will depend on the standard library.

bin/host/Cargo.toml Outdated Show resolved Hide resolved
bin/host/Cargo.toml Outdated Show resolved Hide resolved
bin/host/src/main.rs Outdated Show resolved Hide resolved
bin/host/src/main.rs Outdated Show resolved Hide resolved
bin/host/src/main.rs Outdated Show resolved Hide resolved
bin/host/Cargo.toml Outdated Show resolved Hide resolved
# workspace
anyhow.workspace = true
tracing.workspace = true
alloy-primitives.workspace = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Activate the serde feature so that B256 and other primitives from alloy implement serde::Serialize

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can do this by just follow what the other crates do by creating a "serde" feature and a default feature that automatically enables serde.

Then have that serde feature have "alloy-primitives/serde" in it's list which enables the serde feature on alloy-primitives

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah for sure - given that this is a bin crate, think we can just always enable serde. Folks shouldn't be consuming this code downstream.

@clabby clabby added K-feature Kind: feature A-host Area: host binary labels Apr 24, 2024
parser::parse_b256,
types::{Network, RpcKind}
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Comment on lines 1 to 2
mod parser;
mod types;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should go below use directives

Cargo.toml Outdated
@@ -15,7 +15,7 @@ exclude = ["**/target", "benches/", "tests"]
anyhow = { version = "1.0.79", default-features = false }
tracing = { version = "0.1.40", default-features = false }
cfg-if = "1.0.0"
alloy-primitives = { version = "0.7.0", default-features = false }
alloy-primitives = { version = "0.7.0", default-features = false, features = ["serde"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be enabled by default in the workspace dep - should be enabled in kona-host's Cargo.toml

@refcell refcell added this pull request to the merge queue Apr 25, 2024
Merged via the queue into op-rs:main with commit 05d452b Apr 25, 2024
@github-actions github-actions bot mentioned this pull request Apr 25, 2024
clabby added a commit that referenced this pull request Apr 25, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 25, 2024
This was referenced May 29, 2024
This was referenced Jun 6, 2024
This was referenced Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-host Area: host binary K-feature Kind: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants