Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 5 flow control exceptions #7537

Merged
merged 6 commits into from
Oct 2, 2024
Merged

Remove 5 flow control exceptions #7537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
02cc3a8
Remove two flow control exceptions
benaadams Oct 1, 2024
9d527d9
Precompile out of gas also
benaadams Oct 1, 2024
9bab52a
Also Div32Ceiling outofgas
benaadams Oct 1, 2024
e5f66d5
UpdateMemoryCost also
benaadams Oct 2, 2024
55da1a2
Fix tests
benaadams Oct 2, 2024
7594730
Throw even less exceptions
benaadams Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 63 additions & 11 deletions src/Nethermind/Nethermind.Evm/EvmPooledMemory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,30 @@ public void Save(in UInt256 location, Span<byte> value)
value.CopyTo(_memory.AsSpan((int)location, value.Length));
}

private static void CheckMemoryAccessViolation(in UInt256 location, in UInt256 length, out ulong newLength, out bool outOfGas)
{
if (location.IsLargerThanULong() || length.IsLargerThanULong())
{
outOfGas = true;
newLength = 0;
return;
}

CheckMemoryAccessViolationInner(location.u0, length.u0, out newLength, out outOfGas);
}

private static void CheckMemoryAccessViolation(in UInt256 location, in UInt256 length, out ulong newLength)
{
if (location.IsLargerThanULong() || length.IsLargerThanULong())
{
ThrowOutOfGasException();
}

CheckMemoryAccessViolation(location.u0, length.u0, out newLength);
CheckMemoryAccessViolationInner(location.u0, length.u0, out newLength, out bool outOfGas);
if (outOfGas)
{
ThrowOutOfGasException();
}
}

private static void CheckMemoryAccessViolation(in UInt256 location, ulong length, out ulong newLength)
Expand All @@ -78,17 +94,24 @@ private static void CheckMemoryAccessViolation(in UInt256 location, ulong length
ThrowOutOfGasException();
}

CheckMemoryAccessViolation(location.u0, length, out newLength);
CheckMemoryAccessViolationInner(location.u0, length, out newLength, out bool outOfGas);
if (outOfGas)
{
ThrowOutOfGasException();
}
}

private static void CheckMemoryAccessViolation(ulong location, ulong length, out ulong newLength)
private static void CheckMemoryAccessViolationInner(ulong location, ulong length, out ulong newLength, out bool outOfGas)
{
ulong totalSize = location + length;
if (totalSize < location || totalSize > long.MaxValue)
{
ThrowOutOfGasException();
outOfGas = true;
newLength = 0;
return;
}

outOfGas = false;
newLength = totalSize;
}

Expand Down Expand Up @@ -197,19 +220,23 @@ private void ClearForTracing(ulong size)
}
}

public long CalculateMemoryCost(in UInt256 location, in UInt256 length)
public long CalculateMemoryCost(in UInt256 location, in UInt256 length, out bool outOfGas)
{
outOfGas = false;
if (length.IsZero)
{
return 0L;
}

CheckMemoryAccessViolation(in location, in length, out ulong newSize);
CheckMemoryAccessViolation(in location, in length, out ulong newSize, out outOfGas);
if (outOfGas) return 0;

if (newSize > Size)
{
long newActiveWords = Div32Ceiling(newSize);
long activeWords = Div32Ceiling(Size);
long newActiveWords = Div32Ceiling(newSize, out outOfGas);
if (outOfGas) return 0;
long activeWords = Div32Ceiling(Size, out outOfGas);
if (outOfGas) return 0;

// TODO: guess it would be well within ranges but this needs to be checked and comment need to be added with calculations
ulong cost = (ulong)
Expand All @@ -230,6 +257,17 @@ public long CalculateMemoryCost(in UInt256 location, in UInt256 length)
return 0L;
}

public long CalculateMemoryCost(in UInt256 location, in UInt256 length)
{
long result = CalculateMemoryCost(in location, in length, out bool outOfGas);
if (outOfGas)
{
throw new OutOfGasException();
}

return result;
}

public TraceMemory GetTrace()
{
ulong size = Size;
Expand All @@ -247,11 +285,12 @@ public void Dispose()
}
}

public static long Div32Ceiling(in UInt256 length)
public static long Div32Ceiling(in UInt256 length, out bool outOfGas)
{
if (length.IsLargerThanULong())
{
ThrowOutOfGasException();
outOfGas = true;
return 0;
}

ulong result = length.u0;
Expand All @@ -264,12 +303,25 @@ public static long Div32Ceiling(in UInt256 length)

if (result > int.MaxValue)
{
ThrowOutOfGasException();
outOfGas = true;
return 0;
}

outOfGas = false;
return (long)result;
}

public static long Div32Ceiling(in UInt256 length)
{
long result = Div32Ceiling(in length, out bool outOfGas);
if (outOfGas)
{
ThrowOutOfGasException();
}

return result;
}

private void UpdateSize(in UInt256 location, in UInt256 length, bool rentIfNeeded = true)
{
UpdateSize((ulong)(location + length), rentIfNeeded);
Expand Down
15 changes: 8 additions & 7 deletions src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,10 @@ protected virtual void ExecuteEvmCall(
if (tx.IsContractCreation)
{
// if transaction is a contract creation then recipient address is the contract deployment address
PrepareAccountForContractDeployment(env.ExecutingAccount, spec);
if (!PrepareAccountForContractDeployment(env.ExecutingAccount, spec))
{
goto Fail;
Copy link
Member

Choose a reason for hiding this comment

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

Is this a logic change, instead of throwing we will go to fail?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no. So would have thrown, and been caught and done same thing.

Now doesn't throw, does same thing (will also do same thing if does throw, as are other exceptions possible)

}
}

ExecutionType executionType = tx.IsContractCreation ? ExecutionType.CREATE : ExecutionType.TRANSACTION;
Expand Down Expand Up @@ -616,14 +619,16 @@ protected virtual void PayFees(Transaction tx, BlockHeader header, IReleaseSpec
}
}

protected void PrepareAccountForContractDeployment(Address contractAddress, IReleaseSpec spec)
protected bool PrepareAccountForContractDeployment(Address contractAddress, IReleaseSpec spec)
{
if (WorldState.AccountExists(contractAddress) && contractAddress.IsNonZeroAccount(spec, _codeInfoRepository, WorldState))
{
if (Logger.IsTrace) Logger.Trace($"Contract collision at {contractAddress}");

ThrowTransactionCollisionException();
return false;
}

return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down Expand Up @@ -658,10 +663,6 @@ protected virtual long Refund(Transaction tx, BlockHeader header, IReleaseSpec s
[DoesNotReturn]
[StackTraceHidden]
private static void ThrowInvalidDataException(string message) => throw new InvalidDataException(message);

[DoesNotReturn]
[StackTraceHidden]
private static void ThrowTransactionCollisionException() => throw new TransactionCollisionException();
}

public readonly struct TransactionResult(string? error)
Expand Down
Loading