EIP-6780: SELFDESTRUCT only in same transaction

[smartprogrammer93]

Closes #4650

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Testing

Requires testing

• Yes
• No

In case you checked yes, did you write tests??

• Yes
• No

[Demuirgos]

if you added a new value Instruction.SELLALL in the instruction enum with the same value of Instruction.SELFDESTRUCT , and in the switch statement you make the case like this :

switch(instruction) {
    ...
    case Instruction.SelfDestruct | Instruction.SellAll :
        Body with your changes
    ...
}

I think this way you wouldn't need to change all the old test files

[smartprogrammer93]

@Demuirgos that did not work since both constants will be the same value. the constant name is same.

[Demuirgos]

@smartprogrammer93, this is close to what I had in mind :

using System;
var opcode = (Instruction)0xff; //op code is name agnostic so it only depends on the value

test(opcode, activateShanghai : true); // we pretend test it with "shanghai" activated
// console result : "SELLALL behavior"
test(opcode, activateShanghai : false);// we pretend test it with "shanghai" deactivated
// console result : "SELFDESTRUCT behavior"

void test(Instruction instruction, bool activateShanghai) {
    switch(instruction) {
        case Instruction.SELLALL | Instruction.SELFDESTRUCT: // since they are both the same value underneath "or"ing them will result in the same value as themselves 
            if(activateShanghai) { // shanghai specific behaviour
                Console.WriteLine("SELLALL behavior");
            } else { // old behaviour
                Console.WriteLine("SELFDESTRUCT behavior");
            }
            break;
        default : 
            break;
    }
}

enum Instruction {
    SELFDESTRUCT = 0xff, // old instruction
    SELLALL = 0xff // new instruction with same value as old instruction
}

[smartprogrammer93]

@Demuirgos Good idea. thank you

[LukaszRozmej]

Not entirely sure if there isn't anything more to it?

[MarekM25]

@smartprogrammer93 we're renaming opcode for the second time in Nethermind history ;) We did it for the merge and DIFFICULTY/PREVRANDAO opcode. And we had to add one more thing to handle traces: https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.Evm/Instruction.cs#L192

Check this out, I believe you need to do similar thing. It is not consensus critical, but it could be easy to forget later.

[Demuirgos]

Can you document these tests a bit, to make it easy to understand the trick being used ?

[Demuirgos]

same here

[Demuirgos]

what if you used the ADDRESS opcode, and why use PrivateKeyB?

[MarekM25]

QQ: no need to change tracing: ITxTracer.ReportSelfDestruct?

[LukaszRozmej]

I see you are not creating/restoring snapshot on _createList, while using parent collection. This can cause issues as created items will bubble up to parent context?

[smartprogrammer93]

I want it to bubble up. I want a list of created contracts in the whole transaction not only the current context