argumentcomputer / zk-light-clients

A collection of ZK light client libraries for various blockchains. (contact: @tchataigner)
23 stars 2 forks source link

CI: Update cycle-count-regression for latest Sphinx #164

Closed wwared closed 1 week ago

wwared commented 1 month ago

After we merge #161, the new version of Sphinx has slightly different output, which causes the regression CI check to fail, see https://github.com/argumentcomputer/zk-light-clients/actions/runs/10411627928/job/28868538903 for an example.

I'm not sure if the total number of cycles is still being printed by the execution-only mode by default anymore, specifically in that CI run it printed for the execute portion:

2024-08-16T16:20:10.576643Z DEBUG execute: loading memory image
2024-08-16T16:20:10.580254Z  INFO execute: clk = 0 pc = 0x201e8c    
2024-08-16T16:20:10.580339Z DEBUG execute: ┌╴read_inputs    
2024-08-16T16:20:10.590888Z  INFO execute: └╴155,189 cycles    
2024-08-16T16:20:10.641776Z DEBUG execute: ┌╴verify_transaction_inclusion    
2024-08-16T16:20:10.643710Z  INFO execute: └╴28,167 cycles    
2024-08-16T16:20:10.643749Z DEBUG execute: ┌╴verify_signature    
2024-08-16T16:20:11.319562Z  INFO execute: └╴7,366,941 cycles    
2024-08-16T16:20:11.319940Z DEBUG execute: ┌╴verify_merkle_proof    
2024-08-16T16:20:11.323110Z  INFO execute: └╴47,397 cycles    
2024-08-16T16:20:11.371492Z  INFO execute: close time.busy=802ms time.idle=1.20µs
Execution took 7.410766367s
test inclusion::test::test_execute_inclusion ... ok

If needed, we can make a small patch to sphinx to re-add this information at the end of execute. This information is still being printed by the prove_core output, so we can also switch the regression check to do that instead as an alternative

samuelburnham commented 1 week ago

Closing as this was fixed in #188

Successful runs: https://github.com/argumentcomputer/zk-light-clients/actions/runs/10831133987/job/30052438529 https://github.com/argumentcomputer/zk-light-clients/actions/runs/10831133987/job/30052440405