fix: remove ineffective witness assignment

[gballet]

I noticed that there was an ineffectual witness creation, as well as a misplaced check for the existence of the witness, I am removing them for cleanup - they clash with some changes introduced by FILL_COST

[gballet]

this stuff is recreated in applyTransaction and isn't needed before, so this is a redundant creation.

[jsign]

I've checked all references to NewEVM and there're many places that call this method that aren't applyTransaction.

I feel it would be much safer to maybe move that initialization inside NewEVMTxContext(...)? I'm not sure why it makes sense to initialize one of its attributes outside the ctor creating these kind of situations.

Also noted that that referenced line doesn't do the chainConfig.IsPrague(blockCtx.BlockNumber, blockCtx.Time) style check that we did here. I guess that's just creating a little extra garbage for block ranges that the witness doesn't apply, but just mentioning.

If we're always fine with doing it ignoring the right fork (as done today in applyTransaction), then moving it to NewEVMTxContext is possible since that method doesn't have block num.

[jsign]

LGMT, but left some comments if we can simplify further.

[jsign]

I had the feeling this removed logic was intentionally trying to lazily initialize in some cases.

Thing is that if we remove this, we could panic "for a while" but eventually this method will have to change to returning *AccessWitnes, error. (Or avoid the if check?)

[gballet]

yes the if check should not be necessary, I am just setting the panic to catch a rebase issue. I'll add a TODO here.

[jsign]

I've checked all references to NewEVM and there're many places that call this method that aren't applyTransaction.

I feel it would be much safer to maybe move that initialization inside NewEVMTxContext(...)? I'm not sure why it makes sense to initialize one of its attributes outside the ctor creating these kind of situations.

Also noted that that referenced line doesn't do the chainConfig.IsPrague(blockCtx.BlockNumber, blockCtx.Time) style check that we did here. I guess that's just creating a little extra garbage for block ranges that the witness doesn't apply, but just mentioning.

If we're always fine with doing it ignoring the right fork (as done today in applyTransaction), then moving it to NewEVMTxContext is possible since that method doesn't have block num.

[gballet]

Suggested change

	panic("witness wasn't initialized")
	// TODO this is meant to catch rebase errors, replace with error return when stabilized
	panic("witness wasn't initialized")