Skip to content
This repository was archived by the owner on May 3, 2024. It is now read-only.

feat: support alpha3 testnet#91

Open
johntaiko wants to merge 23 commits into
taiko-pi-testfrom
feat/a3_pi
Open

feat: support alpha3 testnet#91
johntaiko wants to merge 23 commits into
taiko-pi-testfrom
feat/a3_pi

Conversation

@johntaiko

Copy link
Copy Markdown
Member

No description provided.

@Brechtpd Brechtpd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Still have to go through some parts of the circuits, but looking good!

Comment thread bus-mapping/Cargo.toml
Comment thread external-tracer/src/lib.rs Outdated
Comment thread geth-utils/build.rs
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
Comment on lines +571 to +577
let rpi_rlc_acc_cell = rpi_rlc_acc_cell.unwrap();
rpi_rlc_acc_cell.copy_advice(
|| "keccak(rpi)_input",
region,
self.rpi,
keccak_row,
)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like this logic with the cells and stuff can can be simplified a bit because hashing is always done at the same position, so just enable the keccak lookup on the last row which already contains all the necessary data and good to go?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I need use two cell to store the keccak_input_rlc and keccak_output_rlc, so I reuse the self.rpi and self.rpi_rlc_acc which previously used to store rpi and rpi_rlc, the rpi_rlc here as the keccak_input_rlc for keccak lookup
So, if I don't copy from the rpi_rlc column into the rpi column, I need allocate an new column for keccak_output_rlc

Comment thread zkevm-circuits/src/pi_circuit2.rs Outdated
@johntaiko johntaiko linked an issue May 11, 2023 that may be closed by this pull request
@johntaiko

johntaiko commented May 15, 2023 via email

Copy link
Copy Markdown
Member Author

@Brechtpd

Copy link
Copy Markdown

Can you run formatting and clippy, github shows a lot of clippy warnings/errors.

@johntaiko

Copy link
Copy Markdown
Member Author

Can you run formatting and clippy, github shows a lot of clippy warnings/errors.

Yeah, but some warnings/errors have existed before. e.g. clippy warnings in bus-mapping.
The CI always checks all code instead of changed code.
https://github.com/taikoxyz/zkevm-circuits/pull/91/checks?check_run_id=13536737282

@johntaiko

johntaiko commented May 17, 2023

Copy link
Copy Markdown
Member Author

after the discussion with @smtmfft , because this PR is based on the OLD branch from PSE, we just ignore some warnings/errors. Next testnet, we will use the newest code and let all CI passed

Is this OK?

@Brechtpd

Copy link
Copy Markdown

Ah yeah makes sense.

@ggkitsas ggkitsas left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code-wise LGTM on a first, shallow pass. I am missing some context though, is there a list of changes we need for alpha3?

xiaodino pushed a commit that referenced this pull request Aug 10, 2023
### Description

Remove geth-utils and examples.

### Type of change

New feature (non-breaking change which adds functionality)

### Rationale

- Assembly was introduced in #91 to build examples to use geth-util. It
was not used elsewhere in the codebase.
- The examples are removed as we currently don't use geth-util like the
way the examples show. (Note that the examples also fail to run now. The
transactions in the examples would fail because the trace config
defaults to a 0 block gas limit. It runs after adding a block gas
limit.)
- I found those examples unused when reviewing how we use the tracing
API. It is much easier to review the tracing code after removing the
Assembly.
- The Readme mentioned a TODO CLI, which we never built. I think it
might be interesting to create a good first issue for people to extend
testool(https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/690b2f143a94b7474f0c99493a99b0f2c53d93b8/testool/README.md#L38)
for it.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update public input circuit for new protocol release

3 participants