-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix(optimism): Node Tests #47
Conversation
use reth_primitives::TransactionSigned; | ||
let provider = MockEthProvider::<TransactionSigned>::default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes stink but i think it's unavoidable, even though i hid the op impls behind the optimism feature flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventually we want to remove those op feature flags anyway, I don't think it stinks so bad :)
use reth_primitives::TransactionSigned; | ||
let provider = MockEthProvider::<TransactionSigned>::default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventually we want to remove those op feature flags anyway, I don't think it stinks so bad :)
@@ -164,6 +164,26 @@ impl StateCommitmentProvider for MockEthProvider { | |||
type StateCommitment = <MockNode as NodeTypes>::StateCommitment; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type StateCommitment = <MockNode as NodeTypes>::StateCommitment; | |
impl<T> StateCommitmentProvider for MockEthProvider<T> { | |
type StateCommitment = <MockNode as NodeTypes>::StateCommitment; |
this is instead fixed by generalising the impls. in some cases you may need trait bound T: reth_primitive_traits::SignedTransaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is what we want to upstream, but I'm merging this pr into the base branch anyway to unblock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened an issue paradigmxyz#14038
Description
Fixes node tests in the transaction pool using the updated
OpTransactionSigned
. This requires implementing traits on theMockEthProvider
with the concrete generic instead of the defaultT = TransactionSigned
generic.Closes #45