Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ECDSA/k256 for sov signatures #1798

Open
wants to merge 39 commits into
base: nightly
Choose a base branch
from

Conversation

ercecan
Copy link
Member

@ercecan ercecan commented Feb 3, 2025

Description

Uses k256 after fork2 instead of dalek signatures

With this pr we will be getting rid of sha 512 after fork2 and we will be using k256 signatures.
Main reason to do this is ABOUT 10% our cycles are coming form Dalek signatures and sha512, and risc0 has k256 accelerator so our cycles will drop quite a bit.

Linked Issues

Benchmark

Some stats with 3055 txs in around 15 blocks:

Nightly:

2025-02-10T12:42:51.423059Z  INFO risc0_zkvm::host::server::exec::executor: execution time: 1082.7129585s
2025-02-10T12:42:51.423075Z  INFO risc0_zkvm::host::server::session: number of segments: 56477
2025-02-10T12:42:51.423077Z  INFO risc0_zkvm::host::server::session: 59220426752 total cycles
2025-02-10T12:42:51.423078Z  INFO risc0_zkvm::host::server::session: 14559782266 user cycles (24.59%)
2025-02-10T12:42:51.423080Z  INFO risc0_zkvm::host::server::session: 44439204940 paging cycles (75.04%)
2025-02-10T12:42:51.423081Z  INFO risc0_zkvm::host::server::session: 221439546 reserved cycles (0.37%)
2025-02-10T12:42:51.423083Z  INFO risc0_zkvm::host::server::session: ecalls
2025-02-10T12:42:51.423084Z  INFO risc0_zkvm::host::server::session:    4690797 BigInt2 calls, 620745956 cycles, (1.05%)
2025-02-10T12:42:51.423125Z  INFO risc0_zkvm::host::server::session:    5263758 Sha2 calls, 390848988 cycles, (0.66%)
2025-02-10T12:42:51.423127Z  INFO risc0_zkvm::host::server::session:    8147071 BigInt calls, 81470710 cycles, (0.14%)
2025-02-10T12:42:51.423128Z  INFO risc0_zkvm::host::server::session:    629879 Software calls, 9674036 cycles, (0.02%)
2025-02-10T12:42:51.423130Z  INFO risc0_zkvm::host::server::session:    0 Input calls, 0 cycles, (0.00%)
2025-02-10T12:42:51.423131Z  INFO risc0_zkvm::host::server::session: syscalls
2025-02-10T12:42:51.423132Z  INFO risc0_zkvm::host::server::session:    610016 Keccak calls
2025-02-10T12:42:51.423133Z  INFO risc0_zkvm::host::server::session:    17951 Read calls
2025-02-10T12:42:51.423134Z  INFO risc0_zkvm::host::server::session:    932 VerifyIntegrity calls
2025-02-10T12:42:51.423178Z  INFO risc0_zkvm::host::server::session:    932 ProveKeccak calls
2025-02-10T12:42:51.423179Z  INFO risc0_zkvm::host::server::session:    45 Write calls
WARNING: Proving in dev mode does not generate a valid receipt. Receipts generated from this process are invalid and should never be used in production.
Execution stats: SessionStats { segments: 56477, total_cycles: 59220426752, user_cycles: 14559782266, paging_cycles: 44439204940, reserved_cycles: 221439546 }
test bitcoin::guest_cycles::guest_cycles ... ok

this pr:

025-02-10T09:11:07.813202Z  INFO risc0_zkvm::host::server::exec::executor: execution time: 1070.259843125s
2025-02-10T09:11:07.813231Z  INFO risc0_zkvm::host::server::session: number of segments: 54645
2025-02-10T09:11:07.813233Z  INFO risc0_zkvm::host::server::session: 57299435520 total cycles
2025-02-10T09:11:07.813234Z  INFO risc0_zkvm::host::server::session: 14292721998 user cycles (24.94%)
2025-02-10T09:11:07.813237Z  INFO risc0_zkvm::host::server::session: 42792508176 paging cycles (74.68%)
2025-02-10T09:11:07.813238Z  INFO risc0_zkvm::host::server::session: 214205346 reserved cycles (0.37%)
2025-02-10T09:11:07.813240Z  INFO risc0_zkvm::host::server::session: ecalls
2025-02-10T09:11:07.813241Z  INFO risc0_zkvm::host::server::session:    4777408 BigInt2 calls, 632208982 cycles, (1.10%)
2025-02-10T09:11:07.813278Z  INFO risc0_zkvm::host::server::session:    5263958 Sha2 calls, 391898884 cycles, (0.68%)
2025-02-10T09:11:07.813281Z  INFO risc0_zkvm::host::server::session:    7781511 BigInt calls, 77815110 cycles, (0.14%)
2025-02-10T09:11:07.813283Z  INFO risc0_zkvm::host::server::session:    630006 Software calls, 9674553 cycles, (0.02%)
2025-02-10T09:11:07.813284Z  INFO risc0_zkvm::host::server::session:    0 Input calls, 0 cycles, (0.00%)
2025-02-10T09:11:07.813286Z  INFO risc0_zkvm::host::server::session: syscalls
2025-02-10T09:11:07.813288Z  INFO risc0_zkvm::host::server::session:    610016 Keccak calls
2025-02-10T09:11:07.813289Z  INFO risc0_zkvm::host::server::session:    18078 Read calls
2025-02-10T09:11:07.813290Z  INFO risc0_zkvm::host::server::session:    932 VerifyIntegrity calls
2025-02-10T09:11:07.813355Z  INFO risc0_zkvm::host::server::session:    932 ProveKeccak calls
2025-02-10T09:11:07.813358Z  INFO risc0_zkvm::host::server::session:    45 Write calls
WARNING: Proving in dev mode does not generate a valid receipt. Receipts generated from this process are invalid and should never be used in production.
Execution stats: SessionStats { segments: 54645, total_cycles: 57299435520, user_cycles: 14292721998, paging_cycles: 42792508176, reserved_cycles: 214205346 }
test bitcoin::guest_cycles::guest_cycles ... ok

with 3055 txs in around 15 blocks we can see ~3.5% drop in cycle counts, this drop will be much more in real scenarios when txs are more spread to more blocks, instead 3000 txs being compressed to 15 blocks like in this case.

@ercecan ercecan requested a review from eyusufatik as a code owner February 3, 2025 15:42
@auto-assign auto-assign bot requested a review from rakanalh February 3, 2025 15:42
@ercecan ercecan changed the title Erce/use k256 for sov signatures Use ECDSA/k256 for sov signatures Feb 3, 2025
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 55.57656% with 470 lines in your changes missing coverage. Please review.

Project coverage is 75.9%. Comparing base (6f326d9) to head (8f714ea).
Report is 1 commits behind head on nightly.

Files with missing lines Patch % Lines
crates/sequencer/src/runner.rs 54.7% 85 Missing ⚠️
...k/module-system/sov-modules-api/src/transaction.rs 28.5% 75 Missing ⚠️
...module-system/sov-modules-stf-blueprint/src/lib.rs 27.9% 67 Missing ⚠️
crates/batch-prover/src/runner.rs 35.5% 38 Missing ⚠️
crates/fullnode/src/runner.rs 35.5% 38 Missing ⚠️
crates/fullnode/src/da_block_handler.rs 2.7% 36 Missing ⚠️
crates/batch-prover/src/da_block_handler.rs 44.4% 25 Missing ⚠️
...le-system/sov-modules-api/src/default_signature.rs 80.0% 23 Missing ⚠️
crates/batch-prover/src/proving.rs 21.4% 22 Missing ⚠️
...m/module-implementations/sov-accounts/src/hooks.rs 52.7% 17 Missing ⚠️
... and 10 more
Additional details and impacted files
Files with missing lines Coverage Δ
bin/citrea/src/rollup/mod.rs 79.4% <ø> (ø)
crates/batch-prover/src/lib.rs 100.0% <100.0%> (ø)
crates/citrea-stf/src/genesis_config.rs 100.0% <100.0%> (ø)
crates/citrea-stf/src/hooks_impl.rs 38.4% <ø> (ø)
crates/common/src/config.rs 94.2% <100.0%> (+<0.1%) ⬆️
...ull-node/db/sov-db/src/schema/types/batch_proof.rs 49.3% <100.0%> (-17.9%) ⬇️
...m/module-implementations/sov-accounts/src/tests.rs 98.4% <100.0%> (+0.1%) ⬆️
...dule-system/sov-modules-api/src/default_context.rs 56.8% <100.0%> (+6.8%) ⬆️
...ign-sdk/module-system/sov-modules-api/src/hooks.rs 79.5% <ø> (ø)
...reign-sdk/module-system/sov-modules-api/src/lib.rs 100.0% <ø> (ø)
... and 28 more

... and 2 files with indirect coverage changes

@@ -1,5 +1,6 @@
[public_keys]
sequencer_public_key = "4682a70af1d3fae53a5a26b682e2e75f7a1de21ad5fc8d61794ca889880d39d1"
# TODO: Add sequencer_k256_public_key
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO

Copy link
Member Author

Choose a reason for hiding this comment

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

bin/citrea/src/rollup/mod.rs Outdated Show resolved Hide resolved
Comment on lines +175 to +190
SignedSoftConfirmation::new(
signed_soft_confirmation.l2_height(),
signed_soft_confirmation.hash(),
signed_soft_confirmation.prev_hash(),
signed_soft_confirmation.da_slot_height(),
signed_soft_confirmation.da_slot_hash(),
signed_soft_confirmation.da_slot_txs_commitment(),
signed_soft_confirmation.l1_fee_rate(),
signed_soft_confirmation.blobs().to_vec().into(),
parsed_txs.into(),
signed_soft_confirmation.deposit_data().to_vec(),
signed_soft_confirmation.signature().to_vec(),
signed_soft_confirmation.pub_key().to_vec(),
signed_soft_confirmation.timestamp(),
)
};
Copy link
Member

Choose a reason for hiding this comment

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

try to get rid of this

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if possible

because I would need to do from SignedSoftConfirmation<PreForkTransaction> for SignedSoftConfirmation

I can not do this outside of the crate, I cannot do this in the crate because I do not have access to those concrete types
so pass I think

crates/sequencer/src/runner.rs Show resolved Hide resolved
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.

SoftConfirmation and Sov-Tx's uses ECDSA signatures
3 participants