Skip to content

Conversation

@Evalir
Copy link
Member

@Evalir Evalir commented Sep 21, 2023

Migrates forge's primitive types to alloy. Types remaining from ethers are solc, etherscan and rpc related unless highlighted for discussion.

@Evalir Evalir requested review from DaniPopes and mattsse September 21, 2023 20:07
@Evalir Evalir changed the title feat(forge): partial alloy primitives migration feat(forge): partial forge migration Sep 21, 2023
use alloy_primitives::{Address, Bytes, U256};
use ethers_addressbook::contract;
use ethers_core::types::*;
use ethers_core::types::{BlockId, Chain, NameOrAddress};
Copy link
Member Author

Choose a reason for hiding this comment

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

Wonder what we wanna do with this BlockId type? We also have BlockNumber which is a loosely related struct and I feel we could do some type consolidation. This probably concerns rpc types

Copy link
Member

@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.

lgtm,
clarification on B256

/// A collection of [HitMap]s
#[derive(Default, Clone, Debug)]
pub struct HitMaps(pub HashMap<H256, HitMap>);
pub struct HitMaps(pub HashMap<B256, HitMap>);
Copy link
Member

Choose a reason for hiding this comment

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

why are we using B256 now?
isn't there an alias for H256?
this can be super confusing

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

H stands for Hash which we moved away from months ago :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, @mattsse i think the alias might be local to reth if you're using that there?

@Evalir Evalir merged commit 862bba5 into master Sep 22, 2023
@Evalir Evalir deleted the evalir/forge-core-migration branch September 22, 2023 16:28
@Evalir Evalir mentioned this pull request Sep 25, 2023
43 tasks
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.

4 participants