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

Suspicious Funding V2 Update #642

Open
wants to merge 8 commits into
base: @v2-bots
Choose a base branch
from
Open

Suspicious Funding V2 Update #642

wants to merge 8 commits into from

Conversation

Vxatz
Copy link
Contributor

@Vxatz Vxatz commented Apr 3, 2024

No description provided.

@Vxatz Vxatz self-assigned this Apr 3, 2024
@Vxatz Vxatz requested a review from RCantu92 April 3, 2024 10:36
@Vxatz Vxatz changed the base branch from main to @v2-bots April 3, 2024 10:38
@0xtaf 0xtaf self-requested a review April 3, 2024 15:19
Copy link
Collaborator

@0xtaf 0xtaf left a comment

Choose a reason for hiding this comment

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

  • I think as any is an acceptable choice at this point.
  • source is missing in the findings.

@0xtaf
Copy link
Collaborator

0xtaf commented Apr 4, 2024

not notes for v2 but as a performance improvement, would you think the first condition is often false?

      if (
        (await provider.getTransactionCount(txEvent.to)) === 0 &&
        (await provider.getCode(txEvent.to)) === "0x"
      ) {

I was thinking to use Promise.all but if the first condition is generally false, this is more efficient as the second condition wouldn't be evaluated.

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