Skip to content

refactor[contracts]: Port OVM_ECDSAContractAccount to use ovm-solc#546

Merged
smartcontracts merged 4 commits intomasterfrom
ovm-solc/ecdsa-contract-account
Apr 22, 2021
Merged

refactor[contracts]: Port OVM_ECDSAContractAccount to use ovm-solc#546
smartcontracts merged 4 commits intomasterfrom
ovm-solc/ecdsa-contract-account

Conversation

@smartcontracts
Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts commented Apr 21, 2021

Description
Another continuation of #475. Ports OVM_ECDSAContractAccount to use ovm-solc. Pretty straightforward PR.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 21, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: 21fdc8d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/contracts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@smartcontracts smartcontracts marked this pull request as ready for review April 21, 2021 22:46
karlfloersch
karlfloersch approved these changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@karlfloersch karlfloersch left a comment

Choose a reason for hiding this comment

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

Asked a question here, and @ben-chain you should sign off as well, but overall this looks great!

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol
if (decodedTx.to == address(0)) {
(address created, bytes memory revertData) = Lib_SafeExecutionManagerWrapper.safeCREATE(
gasleft(),
(address created, bytes memory revertdata) = Lib_ExecutionManagerWrapper.ovmCREATE(
Copy link
Copy Markdown
Contributor

@karlfloersch karlfloersch Apr 21, 2021

Choose a reason for hiding this comment

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

Why do we need to wrap create?

Copy link
Copy Markdown
Collaborator

@ben-chain ben-chain Apr 21, 2021

Choose a reason for hiding this comment

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

see https://github.com/ethereum-optimism/optimism/pull/475/files#r617100336 for a discussion of the alternative using ovmCREATE. Personally I prefer having the wrapper handle this.

Copy link
Copy Markdown
Contributor Author

@smartcontracts smartcontracts Apr 21, 2021

Choose a reason for hiding this comment

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

Yes, agree with Ben here. Doing create in assembly is ugly :-/ much cleaner to wrap it.

@smartcontracts smartcontracts changed the title Port OVM_ECDSAContractAccount to use ovm-solc refactor[contracts]: Port OVM_ECDSAContractAccount to use ovm-solc Apr 21, 2021
ben-chain
ben-chain approved these changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ben-chain ben-chain left a comment

Choose a reason for hiding this comment

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

I have a minor nit around what interface this uses but feels like a separate PR since it was already this way previously. Approving!

tynes
tynes requested changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

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

Please add a changeset and is this safe to merge to master?

@smartcontracts
Copy link
Copy Markdown
Contributor Author

smartcontracts commented Apr 21, 2021

@tynes Safe to merge.

@smartcontracts smartcontracts merged commit ce5d596 into master Apr 22, 2021
@smartcontracts smartcontracts deleted the ovm-solc/ecdsa-contract-account branch April 22, 2021 18:29
@ben-chain ben-chain mentioned this pull request Apr 23, 2021
Closed
2 tasks
InoMurko referenced this pull request in omgnetwork/optimism May 25, 2021
@smartcontracts
refactor[contracts]: Port OVM_ECDSAContractAccount to use ovm-solc (#546
cce8376 
)

* Port OVM_ECDSAContractAccount to use ovm-solc

* refactor[contracts]: Use OVM_ETH instead of iOVM_ERC20

* chore[contracts]: Add changeset
theochap pushed a commit that referenced this pull request Dec 10, 2025
@clabby
chore(workspace): Bump MSRV to 1.81 (#546)
94df0c4
emhane pushed a commit that referenced this pull request Feb 4, 2026
@dhyaniarun1993 @emhane
fix: support Send in test_op_proofs_state_provider_ref_debug (#546)
f54238a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maurelian maurelian maurelian left review comments

@karlfloersch karlfloersch karlfloersch approved these changes

@tynes tynes tynes approved these changes

@ben-chain ben-chain ben-chain approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@smartcontracts @karlfloersch @tynes @ben-chain @maurelian