Skip to content

Conversation

Brianspha
Copy link
Contributor

@Brianspha Brianspha commented Oct 3, 2025

Issue 1

The revive node currently enforces a hard byte-size limit on the host function that processes LOG data, more specifically, the condition (len > max_value_size()) and halts with InvalidOperandOOG. This diverges from mainnet Ethereum behavior and REVM defaults, where LOG opcodes are limited by gas and memory expansion, not by an explicit byte cap. The cap causes otherwise valid contracts to fail on when deploying evm bytecode to the revive node.

Consider the below scenario

  • On mainnet Ethereum, LOG opcodes charge a base cost, per-topic cost, and per-byte cost for the data region. There is no protocol-level size cap on the data payload. Execution succeeds if the caller can afford gas and memory.

  • REVM follows this model. It validates topic count (≤ 4) and charges gas. It does not impose a maximum data length by default.

  • Inside pallet revive host.rs, log<N> adds:

    if (len as u32) > context.interpreter.extend.max_value_size() {
        context.interpreter.halt(InstructionResult::InvalidOperandOOG);
        return;
    }

    This check triggers before or around the gas accounting. The next line already charge:

    gas!(context.interpreter, RuntimeCosts::DepositEvent { num_topic: N as u32, len: len as u32 });

This is a problem because valid bytecode that emits larger events fails on when deployed to the revive node with InvalidOperandOOG

  • Example from logs:

    LOG4: offset=704 len=640 max_value_size=416 [I added a debug log which surfaced these values]
    → InterpreterResult { result: InvalidOperandOOG }
    
  • This is observable with a minimal reproduction:

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity ^0.8.24;

contract LogShowcase {
    // Three indexed topics; dynamic args go in log data
    event BatchSwapLike(
        address indexed pool,
        address indexed sender,
        address indexed recipient,
        int256[] assetDeltas,
        uint256[] limits
    );

    event Minimal(
        address indexed a,
        address indexed b,
        address indexed c,
        bytes data
    );

    // Balancer-like: two dynamic arrays (data size = 128 + 64*n)
    function emitBatchSwapLike(uint256 n) external {
        int256[] memory deltas = new int256[](n);
        uint256[] memory lims = new uint256[](n);
        emit BatchSwapLike(msg.sender, address(this), address(0), deltas, lims);
    }

    // Minimal high-level: single bytes payload
    function emitMinimalBytes(uint256 payloadLen) external {
        bytes memory payload = new bytes(payloadLen);
        emit Minimal(msg.sender, address(this), address(0), payload);
    }

    // Low-level: raw LOG3 with exact data length `len`
    function emitRawLog3(uint256 len) external {
        bytes memory buf = new bytes(len);
        assembly {
            log3(add(buf, 32), len, caller(), address(), 0)
        }
    }

    function abiEventDataSize_BatchSwapLike(uint256 n) external pure returns (uint256) {
        return 128 + 64 * n;
    }

    function abiEventDataSize_Minimal(uint256 payloadLen) external pure returns (uint256) {
        uint256 padded = ((payloadLen + 31) / 32) * 32;
        return 64 + padded;
    }
}

What shpuld be happening is:

  • Parity with mainnet EVM and REVM: LOG opcodes succeed if gas and memory allow. No fixed byte cap. Larger logs pay proportionally more gas. Tooling and dapps expect this.

What is actually happening

  • Over-cap emits halt with InvalidOperandOOG

The impact of this cap it breaks portability of contracts and tests that are otherwise conformant with Ethereum, preventing certain standard patterns (batch events, structured audit logs, and larger metadata) from working and and and and

Ideally, we would want to as a suggestion for TESTING:

Remove the cap

  • Delete the max_value_size check.

  • Keep existing metering:

    gas!(context.interpreter, RuntimeCosts::DepositEvent { num_topic: N as u32, len: len as u32 });

Issue 2

[Add description tomorrow]

Issue 3

[Add description sometime later]

scc branch used: https://github.com/paritytech/scc-testing/tree/spha/minor-fix

Note

The explanation above still needs work but posting for visibility

@Brianspha Brianspha marked this pull request as draft October 3, 2025 01:43
@Brianspha Brianspha requested a review from bee344 October 3, 2025 01:43
@Brianspha Brianspha changed the title [Add Title] [Add Title]: WIP Oct 3, 2025
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18210438123
Failed job name: test-linux-stable

@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18210438123
Failed job name: test-linux-stable-int

@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18210438123
Failed job name: test-linux-stable-no-try-runtime

@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18210438123
Failed job name: test-linux-stable

@Brianspha Brianspha changed the title [Add Title]: WIP [Add Title Multiple issues to repor]: WIP Oct 3, 2025
@Brianspha Brianspha changed the title [Add Title Multiple issues to repor]: WIP [Add Title Multiple issues to report]: WIP Oct 3, 2025
@Brianspha Brianspha changed the title [Add Title Multiple issues to report]: WIP Multiple issues Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant