SELF-DESTRUCT and minimizing git conflicts when rebasing from REVM

[PaulRBerg]

We have a problem regarding SELF-DESTRUCT.

We want to disable SELFDESTRUCT because it significantly complicates the MNTs implementation, i.e., what happens when an account holding a lot of MNTs is destructed?

Since SELFDESTRUCT is going to be partially disabled in CANCUN, it's fine to completely disable the opcode in the SabVM. However, it does not follow that we should remove all code references to SELFDESTRUCT because we still need to keep rebasing from REVM. We need to make a choice between:

1. Remove all code references to SELFDESTRUCT
2. Do not remove all code references, only disable them in the entry points.

@IaroslavMazur went with the first approach in #14. However, as discussed in person today, I am of a different view, namely, that we should go with the second approach.

To keep things simple for now, I am happy to leave the PR implementation as is. But I do want us to monitor the technical debt cost introduced by this complete removal of SELFDESTRUCT.

That is, if in a future rebase from REVM, we notice that there are lots of git conflicts, then we switch over to the second approach. Alternatively, we keep using the first.

[IaroslavMazur]

Agree to bring back the SELFDESTRUCT-related logic back if the git conflicts we'll need to be resolving at each rebase onto revm (which will likely happen regularly) will prove to be too troublesome.🫡

Alternatively, we could also be merging SabVM with (and not rebasing it onto) revm. This will, of course, lead to a way "dirtier" project history, but should resolve the issue of the repetitive git conflicts.
Personally, I'm more of a rebase guy, but have felt that this option should, at least, be mentioned in this discussion.

[PaulRBerg]

I also like rebasing, but that's an interesting idea with merging.

[PaulRBerg]

Any updates on this, @IaroslavMazur?

I recall you mentioned on the all-hands call that you've been through some ugly rebases with REVM.

Would it have helped if the SELF-DESTRUCT logic had remained in SabVM?

[IaroslavMazur]

Would it have helped if the SELF-DESTRUCT logic had remained in SabVM?

Of course, it would.

However, together with the more straightforward rebases/merges, not removing the SELFDESTRUCT logic from the SabVM codebase would, potentially, mean poorer DX.

We'd have to touch the respective logic, anyways, as, for example, the value inside the destructed contract must be transferred elsewhere during the destruction. Therefore, we'd have to adapt it for the MNTs workflow - even though we're sure these adaptations make no sense and will be removed in the future.

In other words, we couldn't avoid git conflicts here altogether.

At the same time, the SELFDESTRUCT-related conflicts have been relatively easy to resolve, because they're of the "modified/deleted" type, and given that we don't want to have SELFDESTRUCT, we can simply remove the associated pieces of code (modified in revm, but deleted on our side) altogether. We just need to make sure we're not deleting more/less than is needed.

---

Side idea: only merges (and not rebases) will, probably, be justified for SabVM in the future. The more so - as more changes pile up in our repo. This is because rebasing continuously, even after having originally integrated our custom changes into the codebase would require us to:

1. keep rebasing all of the SabVM-specific changes (starting from the very first commit - and down to the most recent one),
2. continue doing this all over and over again as revm keeps being developed, potentially resulting in dozens (if not hundreds) of repeated conflicts, and
3. potentially wreck the traceability inside our repo (e.g. what happens if you integrate a PR into main, but, then, go and rebase the main branch? is GitHub smart enough to associate the PR commits (which now have completely different hashes) with the old PR?)

[PaulRBerg]

I see what you mean - and yes, you're right that we couldn't avoid git conflicts in any case.

only merges (and not rebases) will, probably, be justified for SabVM in the future

Makes sense, but I think that at the beginning we should strive to rebase for as long as we can because rebasing would make audits easier. E.g. we could squash our commits as you have suggested elsewhere.

[IaroslavMazur]

rebasing would make audits easier

I must admit I haven't considered the audits before. Thank you for the new perspective!

---

Although, shouldn't this only matter if revm gets audited before SabVM (such that the auditors could, eventually, only audit the changes we made to revm (i.e. in SabVM) after the audit commit)?

revm hasn't been audited yet, if my understanding is correct. Nor will it get audited anytime soon, considering how fast it's still being changed 😅

[PaulRBerg]

Good point that it only matters if REVM is audited.

I'll close this discussion for now.

[PaulRBerg]

As argued here, there's another important reason for wanting to keep the SELF-DESTRUCT logic.

Minimizing differences between SabVM and REVM.