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

Eth: BlockRef #12251

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Eth: BlockRef #12251

merged 1 commit into from
Oct 2, 2024

Conversation

axelKingsley
Copy link
Contributor

Adds an alias for L1BlockRef called BlockRef and uses it throughout the Supervisor.

This is because the supervisor was dealing with a mix of L1 and L2 block references. Even though logically most references are L2, we only need as much data as the L1 reference had, so they were being used interchangeably. This way everything is using the same type.

Copy link
Contributor

semgrep-app bot commented Oct 2, 2024

Semgrep found 8 golang_fmt_errorf_no_params findings:

No fmt.Errorf invocations without fmt arguments allowed

Ignore this finding from golang_fmt_errorf_no_params.

Semgrep found 3 dangerous-exec-command findings:

Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.

Ignore this finding from dangerous-exec-command.

@tynes tynes added this pull request to the merge queue Oct 2, 2024
Merged via the queue into develop with commit 40a70bd Oct 2, 2024
62 checks passed
@tynes tynes deleted the eth/blockref branch October 2, 2024 15:53
@@ -85,6 +85,10 @@ func (id L1BlockRef) ParentID() BlockID {
}
}

// BlockRef is a Block Ref indepdendent of L1 or L2
// Because L1BlockRefs are strict subsets of L2BlockRefs, BlockRef is a direct alias of L1BlockRef
Copy link
Contributor

@geoknee geoknee Oct 2, 2024

Choose a reason for hiding this comment

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

I would suggest describing BlockRef as the "the common properties of L1 and L2 blocks", to avoid any ambiguity where we might expect it to "work" as an L2 block in all cases.

We could also consider embedding BlockRef into L2BlockRef, which could reduce duplication across a lot of the methods. But, this may result in some yak shaving (we would need to update hundreds of cases where we construct a literal L2BlockRef).

samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
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.

3 participants