Fix EXTCODECOPY with empty account

[ed255]

Description

We treat empty accounts by storing their code_hash in the RwTable as 0. EXTCODECOPY was obtaining the bytecode length by querying the bytecode table with code_hash=0 on existing accounts, but that entry should be invalid (there's no bytecode with code_hash=0). Skip the bytecode table length lookup when code_hash=0.

I've also reintroduced the Block::debug_print_txs_steps_rw_ops function, updated to use the new Block::get_rws API. This function is not used in the code, but it's very convenient to call it when debugging.

Issue Link

Resolve #1190

Type of change

• Bug fix (non-breaking change which fixes an issue)

[ChihChengLiang]

LGTM for the addition and the reintroduction of debug function. Feel free to merge.

I noticed that we have cb.bytecode_length in 6 places.

• In CODESIZE and EXTCODESIZE, we look up the bytecode length for the opcode output.
• In CODECOPY and EXTCODECOPY, length determines the destination address in memory.
• In ErrInvalidJUMP, we check if we jump out of the range of the code size.
• In STOP, we check program counter is not out of the range of code size.

For CODESIZE, CODECOPY, ErrInvalidJUMP, and STOP, we can safely assume we don't query empty hash in those cases, not even in the init code.

For EXTCODESIZE and EXTCODECOPY, the empty hash checks look very similar, maybe execution state merging is a possible optimization in the future.

[ed255]

I noticed that we have cb.bytecode_length in 6 places.

* In CODESIZE and EXTCODESIZE, we look up the bytecode length for the opcode output.
* In CODECOPY and EXTCODECOPY, length determines the destination address in memory.
* In ErrInvalidJUMP, we check if we jump out of the range of the code size.
* In STOP, we check program counter is not out of the range of code size.

For CODESIZE, CODECOPY, ErrInvalidJUMP, and STOP, we can safely assume we don't query empty hash in those cases, not even in the init code.

For EXTCODESIZE and EXTCODECOPY, the empty hash checks look very similar, maybe execution state merging is a possible optimization in the future.

Nice summary! Indeed EXTCODESIZE and EXTCODECOPY are the two opcodes that can deal with non-existing bytecode. The code that overlaps between the two is:

bus-mapping code:

        let exists = !account.is_empty();
        let code_hash = if exists {
            account.code_hash
        } else {
            H256::zero()
        };

circuit code:

        /// configure
        let code_hash = cb.query_cell_phase2();
        // For non-existing accounts the code_hash must be 0 in the rw_table.
        cb.account_read(address.expr(), AccountFieldTag::CodeHash, code_hash.expr());
        let not_exists = IsZeroGadget::construct(cb, code_hash.expr());
        let exists = not::expr(not_exists.expr());

        let code_size = cb.query_word_rlc();
        cb.condition(exists.expr(), |cb| {
            cb.bytecode_length(code_hash.expr(), from_bytes::expr(&code_size.cells));
        });
        cb.condition(not_exists.expr(), |cb| {
            cb.require_zero("code_size is zero when non_exists", code_size.expr());
        });
        /// assignment
        self.code_hash
            .assign(region, offset, region.word_rlc(code_hash))?;
        self.not_exists
            .assign_value(region, offset, region.word_rlc(code_hash))?;

I think it's not a lot of code to worry about code deduplication. About merging the two ExecStates, the EXTCODECOPY performs a lot of extra logic that would need to become conditional (potentially increasing the constraints degree, which means introducing new intermediate cells to keep the degree low by the split_expression engine), so maybe it can be explored but I'm not confidence that there's a clear gain.

[ed255]

I have just merged this PR with a single review because it was a very small fix and it doesn't introduce any new pattern.