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

feat(tee): add support for recoverable signatures #3414

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pbeza
Copy link
Collaborator

@pbeza pbeza commented Dec 30, 2024

What ❔

This PR is part of the effort to implement on-chain TEE proof verification. Signatures produced by the TEE Prover are now compatible with the on-chain verifier that uses the ecrecover precompile.

Why ❔

Until now, we've been using non-recoverable signatures in the TEE prover with a compressed ECDSA public key in each attestation – it was compressed because there are only 64 bytes available in the report attestation quote. That worked fine for off-chain proof verification, but for on-chain verification, it's better to use the Ethereum address derived from the public key so we can call ecrecover in Solidity to verify the signature.

This PR goes hand in hand with:

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zkstack dev fmt and zkstack dev lint.

@pbeza pbeza force-pushed the tee/feat/solidity-compatible-recoverable-signatures branch 4 times, most recently from 9c15a1b to 1c239e7 Compare December 30, 2024 17:28
@pbeza pbeza requested review from haraldh and slowli December 30, 2024 17:46
pbeza added a commit to matter-labs/teepot that referenced this pull request Dec 30, 2024
This PR is part of the effort to implement on-chain TEE proof
verification. Signatures produced by the TEE Prover are now compatible
with the on-chain verifier that uses the `ecrecover` precompile.

Until now, we've been using _non-recoverable_ signatures in the TEE
prover with a compressed ECDSA public key in each attestation -- it was
compressed because there are only 64 bytes available in the report
attestation quote. That worked fine for off-chain proof verification,
but for on-chain verification, it's better to use the Ethereum address
derived from the public key so we can call ecrecover in Solidity to
verify the signature.

This PR goes hand in hand with matter-labs/teepot#228
@pbeza pbeza force-pushed the tee/feat/solidity-compatible-recoverable-signatures branch from 1c239e7 to 63a7712 Compare December 31, 2024 10:08
pbeza added a commit to matter-labs/teepot that referenced this pull request Dec 31, 2024
…report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with matter-labs/zksync-era#3414.
pbeza added a commit to matter-labs/teepot that referenced this pull request Dec 31, 2024
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Dec 31, 2024
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Dec 31, 2024
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 2, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 2, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 3, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 3, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 3, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 6, 2025
…report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with matter-labs/zksync-era#3414.
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 6, 2025
…in report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with:
- matter-labs/zksync-era#3414
- #228
@@ -67,10 +67,24 @@ impl fmt::Debug for TeeProver {
}

impl TeeProver {
/// Signs the message in Ethereum-compatible format for on-chain verification.
pub fn sign_message(sec: &SecretKey, message: Message) -> Result<[u8; 65], TeeProverError> {
let s = SECP256K1.sign_ecdsa_recoverable(&message, sec);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the zksync_crypto_primitives library; it has this functionality (and one used in the unit tests below) implemented.

Copy link
Collaborator Author

@pbeza pbeza Jan 14, 2025

Choose a reason for hiding this comment

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

I was excited to hear we had all the crypto primitives ready to reuse, but when I dove in, it turned out they were kinda inconvenient or even impossible to use because:

  1. Some primitives are defined as pub(super), pub(crate), or totally private. I don't really get why, TBH.
  2. Some of our crypto wrappers are way less convenient than the primitives they actually wrap.

I reused what I could, but I still had to almost copy-paste some of the existing primitives. :( Overall it seems the number of LOC increased. :P Lemme know if I'm missing something.

/// Recovers the public key from the signature for the message
fn recover(signature: &Signature, message: &Message) -> Result<Public> {
let rsig = RecoverableSignature::from_compact(
&signature[0..64],
RecoveryId::from_i32(signature[64] as i32 - 27)?,
)?;
let pubkey = &SECP256K1.recover_ecdsa(&Message::from_slice(&message[..])?, &rsig)?;
let serialized = pubkey.serialize_uncompressed();
let mut public = Public::default();
public.as_bytes_mut().copy_from_slice(&serialized[1..65]);
Ok(public)
}
/// Convert public key into the address
fn public_to_address(public: &Public) -> Address {
let hash = keccak256(public.as_bytes());
let mut result = Address::zero();
result.as_bytes_mut().copy_from_slice(&hash[12..]);
result

core/bin/zksync_tee_prover/Cargo.toml Outdated Show resolved Hide resolved
@pbeza pbeza force-pushed the tee/feat/solidity-compatible-recoverable-signatures branch 2 times, most recently from 279292a to c801dd7 Compare January 14, 2025 13:38
@pbeza pbeza force-pushed the tee/feat/solidity-compatible-recoverable-signatures branch from c801dd7 to 27ad86e Compare January 14, 2025 15:16
@pbeza pbeza requested a review from slowli January 14, 2025 15:39
@pbeza pbeza requested a review from haraldh January 14, 2025 15:39
@pbeza
Copy link
Collaborator Author

pbeza commented Jan 14, 2025

@slowli @haraldh, pls re-review. Thanks!

pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 15, 2025
…report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with matter-labs/zksync-era#3414.
pbeza added a commit to matter-labs/teepot that referenced this pull request Jan 15, 2025
…report_data

This PR is part of the effort to implement on-chain TEE proof
verification. This PR goes hand in hand with matter-labs/zksync-era#3414.
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.

3 participants