refactor[contracts]: Remove two unused contracts#549
Conversation
smartcontracts
requested review from
ben-chain,
karlfloersch,
maurelian and
tynes
as code owners
🦋 Changeset detectedLatest commit: a4132c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
ben-chain
left a comment
ben-chain
left a comment
There was a problem hiding this comment.
Ahhhh, very satisfying. 😄
packages/contracts/src/predeploys.ts
Outdated
| OVM_DeployerWhitelist: '0x4200000000000000000000000000000000000002', | ||
| OVM_ECDSAContractAccount: '0x4200000000000000000000000000000000000003', | ||
| OVM_ProxySequencerEntrypoint: '0x4200000000000000000000000000000000000004', | ||
| //OVM_ProxySequencerEntrypoint: '0x4200000000000000000000000000000000000004', // removed |
There was a problem hiding this comment.
Yeah I think this makes sense
There was a problem hiding this comment.
Why comment it out instead of removing it?
There was a problem hiding this comment.
Ah I thought it might be weird if the addresses go from 0x42....03 to 0x42....05 without explanation. Someone might think there's a missing contract there. I figured we could leave this here for posterity until we figure out what to do about predeploys we're no longer using.
There was a problem hiding this comment.
I think it would be better to address with a comment on top of the predeploys object instead of commenting out the line. That will give room to explain why it was removed
There was a problem hiding this comment.
Comment added.
tynes
left a comment
tynes
left a comment
There was a problem hiding this comment.
Please add a changeset and is this safe to merge to master?
|
Yes, safe to merge. edit; under the assumption that both geth and contracts are both upgraded. |
|
The contracts themselves LGTM. |
* refactor[contracts]: Remove two unused contracts * chore[contracts]: Add changeset * docs[contracts]: Added a comment to explain removal of 0x42..05
Follow-up on #548 ## Motivation `OpTransaction` trait can tell if its deposit, but that is not very useful if I also need to extract it. ## Solution Add `as_deposit` function into the trait for the extraction of `TxDeposit`. ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
…550) Follow-up on #549 ## Motivation The RPC transaction object has a dedicated conversion function that accepts `OpTxEnvelope` struct. But it cannot be used with a transaction that is not `OpTxEnvelope` but implements `OpTransaction`. ## Solution Since `OpTransaction` provides all the needed functionality for the conversion, replace the `OpTxEnvelope` type with a generic type. ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
…lloy-rs/op-alloy#550) Follow-up on #549 ## Motivation The RPC transaction object has a dedicated conversion function that accepts `OpTxEnvelope` struct. But it cannot be used with a transaction that is not `OpTxEnvelope` but implements `OpTransaction`. ## Solution Since `OpTransaction` provides all the needed functionality for the conversion, replace the `OpTxEnvelope` type with a generic type. ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
4 participants
Description
Removes two unused contracts,
mockOVM_ECDSAContractAccountandOVM_ProxySequencerEntrypoint.mockOVM_ECDSAContractAccountwas previously being used as a way to simulateeth_callbefore the introduction ofLib_ExecutionManager.simulateMessage.OVM_ProxySequencerEntrypointwas intended to makeOVM_SequencerEntrypointeasily upgradeable but has since been superseded by chugsplash.