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

chore: remove some clones #12008

Merged
merged 1 commit into from
Oct 24, 2024
Merged

chore: remove some clones #12008

merged 1 commit into from
Oct 24, 2024

Conversation

hai-rise
Copy link
Contributor

No description provided.

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm

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.

lgtm,
one pedantic nit

@@ -156,7 +150,7 @@ where
/// a result indicating success with the payload or an error in case of failure.
#[inline]
pub(crate) fn optimism_payload<EvmConfig, Pool, Client>(
evm_config: EvmConfig,
evm_config: &EvmConfig,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not strictly required

Suggested change
evm_config: &EvmConfig,
evm_config: EvmConfig,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, we need this to pass &self.evm_config in from try_build and build_empty_payload instead of cloning anew?

Copy link
Collaborator

Choose a reason for hiding this comment

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

EvmConfig is auto_impl for & so this shouldn't be required by we can also do &

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't work 😅.

error[E0521]: borrowed data escapes outside of method
   --> crates/optimism/payload/src/builder.rs:108:9
    |
104 |         &self,
    |         -----
    |         |
    |         `self` is a reference that is only valid in the method body
    |         let's call the lifetime of this reference `'1`
...
108 |         optimism_payload(&self.evm_config, args, cfg_env, block_env, self.compute_pending_block)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
    |         `self` escapes the method body here
    |         argument requires that `'1` must outlive `'static`

@mattsse mattsse added this pull request to the merge queue Oct 24, 2024
@mattsse mattsse added the A-execution Related to the Execution and EVM label Oct 24, 2024
Merged via the queue into paradigmxyz:main with commit d7f08cd Oct 24, 2024
39 checks passed
@hai-rise hai-rise deleted the rm-clones branch October 24, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-execution Related to the Execution and EVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants