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

changes to keep from HEAD after revert #77

Merged
merged 15 commits into from
Feb 18, 2025
Merged

Conversation

pedrohba1
Copy link
Contributor

@pedrohba1 pedrohba1 commented Feb 14, 2025

This keeps some of the changes in the veemon crate that are not related to the reth revert, while revertinig to use reth as before. The part of reverting to reth is done in https://github.com/semiotic-ai/veemon/tree/revert-to-e13870

One of my concerns here is that reth-primitives TransactioniSigned (below) depends on Signature from alloy_priimitivies which is now marked as deprecated.

pub struct TransactionSigned {
    /// Transaction hash
    pub hash: TxHash,
    /// The transaction signature values
    pub signature: Signature,
    /// Raw transaction info
    #[deref]
    #[as_ref]
    pub transaction: Transaction,
}

In this case, I think we should allow the warnings for now, whiile we convert properly to use alloy only as it was suggested. These deprecated warnings will be removed as soon as PRs like this are mergeed: #74 that make sure examples and tests work properly with alloy are pushed.

A summary of what is kept after reverting to reth:

  • ci setup pages version bump
  • many version bumps in root Cargo.toml that do not affect our code
  • ignores deprecated warnings only in transaction.rs
  • keeps version bumps in header_accumulator and vee, ` from the 41d8900
  • also keeps the changes to header_accumualtor from 0.1.0 to 0.2.0

It does keep reversion of version bumps of decoder, because it keeps previous reth import.

@pedrohba1 pedrohba1 self-assigned this Feb 17, 2025
Copy link
Contributor

@suchapalaver suchapalaver left a comment

Choose a reason for hiding this comment

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

We're on the right track. Please make separate PRs for the separate issues you've identified such as adding a top-level allow deprecated lint and updating the GitHub actions config.

@pedrohba1
Copy link
Contributor Author

removed the version bumps, created the two PRs linked (also removing these PRs code changes)

@pedrohba1
Copy link
Contributor Author

pedrohba1 commented Feb 18, 2025

We're on the right track. Please make separate PRs for the separate issues you've identified such as adding a top-level allow deprecated lint and updating the GitHub actions config.

Just one question, is it better to push the separate PRs in this case into directly main or first into this branch?

(merging the allows deprecated branch here will solve the clippy issue)

@pedrohba1 pedrohba1 merged commit 91e3c80 into revert-to-e13870 Feb 18, 2025
7 of 8 checks passed
@pedrohba1 pedrohba1 deleted the changes-to-keep branch February 18, 2025 16:37
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.

2 participants