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

feat: Alloy type migration #4674

Merged
merged 21 commits into from
Sep 23, 2023
Merged

feat: Alloy type migration #4674

merged 21 commits into from
Sep 23, 2023

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 19, 2023

First commit is 90% automated with regexes

TODO:

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #4674 (68c5aaf) into alloy (a792841) will decrease coverage by 0.17%.
Report is 6 commits behind head on alloy.
The diff coverage is 83.85%.

Impacted file tree graph

Files Changed Coverage Δ
bin/reth/src/args/secret_key.rs 0.00% <ø> (ø)
bin/reth/src/args/utils.rs 75.34% <ø> (ø)
bin/reth/src/chain/import.rs 6.89% <0.00%> (ø)
bin/reth/src/cli/config.rs 0.00% <0.00%> (ø)
bin/reth/src/db/list.rs 1.47% <ø> (ø)
bin/reth/src/debug_cmd/execution.rs 1.12% <0.00%> (ø)
bin/reth/src/node/mod.rs 26.11% <0.00%> (ø)
crates/consensus/auto-seal/src/lib.rs 0.00% <0.00%> (ø)
crates/net/dns/src/tree.rs 86.04% <ø> (ø)
crates/net/eth-wire/src/disconnect.rs 82.66% <ø> (ø)
... and 142 more

... and 11 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.55% <9.47%> (+<0.01%) ⬆️
unit-tests 63.42% <82.69%> (-0.20%) ⬇️

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

Components Coverage Δ
reth binary 32.11% <45.45%> (-0.07%) ⬇️
blockchain tree 83.80% <100.00%> (+0.06%) ⬆️
pipeline 88.53% <92.10%> (ø)
storage (db) 73.76% <98.93%> (+0.06%) ⬆️
trie 94.76% <100.00%> (+0.03%) ⬆️
txpool 50.29% <100.00%> (+<0.01%) ⬆️
networking 77.13% <100.00%> (-0.02%) ⬇️
rpc 57.55% <65.83%> (-0.35%) ⬇️
consensus 63.15% <84.00%> (+0.05%) ⬆️
revm 28.40% <54.54%> (+0.06%) ⬆️
payload builder 8.32% <0.00%> (ø)
primitives 86.35% <91.20%> (-0.21%) ⬇️

@DaniPopes DaniPopes marked this pull request as ready for review September 20, 2023 15:59
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

amazing, only 1 q re Bytes type

this PR is blocked by revm, right?

@@ -836,10 +837,10 @@ impl TransactionSigned {
/// Returns the enveloped encoded transactions.
///
/// See also [TransactionSigned::encode_enveloped]
pub fn envelope_encoded(&self) -> bytes::Bytes {
pub fn envelope_encoded(&self) -> Bytes {
Copy link
Collaborator

Choose a reason for hiding this comment

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

which bytes type is this here?

I'd like to keep bytes::Bytes and not a hex wrapper

Copy link
Member Author

Choose a reason for hiding this comment

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

This is alloy Bytes, I don't see why have multiple types in a public interface

@mattsse mattsse added S-blocked This cannot more forward until something else changes C-debt Refactor of code section that is hard to understand or maintain labels Sep 20, 2023
@DaniPopes
Copy link
Member Author

Blocked by Revm migration, and Alloy release

@gakonst
Copy link
Member

gakonst commented Sep 22, 2023

As discussed, let's no longer block this PR on the releases / merging of the deps, let's drive it to the end / remove ethers from as many places as we can, and merge on Monday after we release alpha.9.

@DaniPopes DaniPopes changed the base branch from main to alloy September 23, 2023 00:02
@DaniPopes
Copy link
Member Author

DaniPopes commented Sep 23, 2023

Merging into alloy branch and continuing there (#4737)

@DaniPopes DaniPopes merged commit ccae4eb into paradigmxyz:alloy Sep 23, 2023
18 checks passed
@DaniPopes DaniPopes deleted the alloy branch September 23, 2023 00:31
@DaniPopes DaniPopes self-assigned this Sep 23, 2023
@DaniPopes DaniPopes added C-enhancement New feature or request and removed S-blocked This cannot more forward until something else changes C-debt Refactor of code section that is hard to understand or maintain labels Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants