CODECOPY opcode

[roynalnaruto]

Specs PR: privacy-ethereum/zkevm-specs#148

TODOs:

• update bytecode circuit to include a Length row per bytecode (as per CODECOPY opcode zkevm-specs#148)
• refactor CodeSize to be fetched from the bytecode circuit
• update bytecode table in specs (fix: update bytecode table specs zkevm-specs#176)

[roynalnaruto]

@han0110 In the specs lookup implementation here, we check if there's one and only one match and return it. But it does allow optional fields.

How can this be done in the Rust impl?

As I see it, this would have to be changed to:

            Self::Bytecode {
                hash,
                tag,
                index,
                opt_is_code,
                value,
            } => {
                if let Some(is_code) = opt_is_code {
                    vec![hash.clone(), tag.clone(), index.clone(), value.clone(), is_code.clone()]
                } else {
                    vec![hash.clone(), tag.clone(), index.clone(), value.clone()]
                }
            }

Also the layout of bytecode table will have to be modified so that is_code column follows the value column. Wdyt?

[han0110]

In spec we try not to annoying developer to provide all of witness to pass a verification, so lookup is made to be like a normal map lookup, then we can use partial value to get the full row.

But in circuit, we have to provide all, so current approach makes sense me. I think we can stick to this now, because there is another idea that can help to make all of these more sense. (see here https://hackmd.io/Pmd8xoZWRVmko8LkRNhABQ)

[roynalnaruto]

Bus-mapping PR added here: scroll-tech#123

[roynalnaruto]

@han0110 @CPerezz @ed255 Would be great if this PR is reviewed, since the specs for CODECOPY have been merged already.

[ChihChengLiang]

Sorry @roynalnaruto for the delay, I'll help review this too.

[han0110]

Only got some nitpicks, others look good to me.

[ChihChengLiang]

LGTM modulo other review comments.

[han0110]

LGTM! It will be nice to see its part of bus-mapping implemented next (it's fine to me to implement it in further PR), then #419 doesn't need to worry about that, thanks!

[roynalnaruto]

It will be nice to see its part of bus-mapping implemented next

@han0110 It's already in progress here: scroll-tech#123.

Once I'm done with the internal call cases for calldataload and calldatacopy (bus-mapping), I will wrap this up.

[roynalnaruto]

In that case, I think either way looks good to me.

I've made the change where Length = 0, Byte = 1 and Padding = 2

[icemelon]

LGTM!

[icemelon]

Thanks for the contribution @roynalnaruto