Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ jobs:
- name: Run tests
run: snforge test --workspace --features fuzzing --fuzzer-runs 200

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Severity 2 — Low: CI runs the expensive Falcon suite redundantly.

The 65 dev-profile Falcon tests run twice (here and in the coverage run at line 67), several above 1e9 L2 gas, serial in one job — and the coverage run doesn't enable falcon_presets_tests, so the Falcon presets (SRC9 routing, upgrade's assert_only_self) report as entirely untested in Codecov anyway. Two further cost sinks: test_fast_ntt_matches_generic_reference_for_every_basis_vector alone costs 7.65B gas (~76M steps) and is single-handedly why the 100M step cap is needed (the boundary/pseudorandom test already covers the interesting cases — sample every 16th basis vector or feature-gate it); and the #[cfg(test)] felt wrapper re-inlines the 31k-statement NTT body, making ~26.7% of the test program duplicate code and a 44 MB test artifact (have it call ntt_falcon512_fast_u16_unchecked and convert instead).


- name: Run Falcon production-path tests
run: snforge test -p openzeppelin_account --release --features falcon_fast_tests falcon_512

- name: Run tests and generate coverage report
run: snforge test --workspace --coverage

Expand Down
9 changes: 9 additions & 0 deletions LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,12 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

---

Third-party notices

Portions of this software (the NTT engine and parts of the Falcon-512 verifier) are
ported from s2morrow (https://github.com/feltroidprime/s2morrow and
https://github.com/starkware-bitcoin/s2morrow),
Copyright (c) 2025 StarkWare Industries Ltd., licensed under the MIT License.
7 changes: 7 additions & 0 deletions Scarb.lock
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Code generated by scarb DO NOT EDIT.
version = 1

[[package]]
name = "corelib_imports"
version = "0.1.3"
source = "registry+https://scarbs.xyz/"
checksum = "sha256:1dbf8982484a6a085a143f2352dfbc716261c50c60551eb03bff5f0167260ad9"

[[package]]
name = "openzeppelin"
version = "4.0.0-alpha.1"
Expand Down Expand Up @@ -39,6 +45,7 @@ dependencies = [
name = "openzeppelin_account"
version = "4.0.0-alpha.1"
dependencies = [
"corelib_imports",
"openzeppelin_interfaces",
"openzeppelin_introspection",
"openzeppelin_test_common",
Expand Down
1 change: 1 addition & 0 deletions Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ keywords = [

[workspace.dependencies]
assert_macros = "2.18.0"
corelib_imports = "0.1.2"
starknet = "2.18.0"
snforge_std = "0.62.1"

Expand Down
10 changes: 10 additions & 0 deletions packages/account/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ This crate provides components to implement account contracts that can be used f
- `EthAccount` validates transactions from signatures over the
[Secp256k1 curve](https://en.bitcoin.it/wiki/Secp256k1).

- `Falcon512ShakeAccount` validates legacy Falcon-512 submission-algorithm signatures
using SHAKE-256 hash-to-point and a verifier-bound product hint.

- `Falcon512ShakeDirectAccount` validates the corresponding hint-free signatures by
recomputing the polynomial product on-chain.

> **WARNING:** The Falcon accounts use immutable keys and a contract-specific felt encoding.
> They do not implement a finalized NIST FN-DSA standard. Key loss, key compromise, or a
> verifier revision requires migration to a new account address.

### Interfaces

- [`ISRC6`](https://docs.openzeppelin.com/contracts-cairo/3.x/api/account#ISRC6)
Expand Down
3 changes: 3 additions & 0 deletions packages/account/Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fmt.workspace = true
scarb.workspace = true

[dependencies]
corelib_imports.workspace = true
starknet.workspace = true
openzeppelin_introspection = { path = "../introspection" }
openzeppelin_interfaces = { path = "../interfaces" }
Expand All @@ -36,6 +37,8 @@ openzeppelin_test_common = { path = "../test_common" }

[features]
fuzzing = []
# Executes the generated production NTT path; requires the release compiler profile.
falcon_fast_tests = []

[lib]

Expand Down
99 changes: 99 additions & 0 deletions packages/account/src/falcon_512.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts for Cairo v4.0.0-alpha.1 (account/src/falcon_512.cairo)

//! Falcon-512 SHAKE account contracts and their verification implementation.
//!
//! These accounts implement the legacy Falcon submission algorithm with SHAKE-256
//! hash-to-point. Their packed felt public keys and signatures are contract-specific;
//! they are not the encoding or finalized behavior of a NIST FN-DSA standard.

pub(crate) mod account;
pub(crate) mod falcon;
pub mod falcon_512_shake;
pub mod falcon_512_shake_direct;
pub(crate) mod hashing;
pub(crate) mod ntt;
pub(crate) mod packing;
pub(crate) mod zq;
use account::Falcon512SignatureVerifier;

pub use falcon_512_shake::Falcon512ShakeAccount;
pub use falcon_512_shake_direct::Falcon512ShakeDirectAccount;
use hashing::hash_to_point::hash_to_point_shake_512;

/// Number of felts in a packed Falcon-512 public key.
pub(crate) const PUBLIC_KEY_FELTS: u32 = 29;

/// Number of felts in a Falcon-512 signature carrying a product hint.
pub(crate) const SIGNATURE_FELTS: u32 = 60;

/// Number of felts in a hint-free Falcon-512 signature.
pub(crate) const DIRECT_SIGNATURE_FELTS: u32 = 31;

/// Verifier for the 60-felt SHAKE-256 signature carrying a polynomial-product hint.
pub(crate) impl Falcon512ShakeVerifier of Falcon512SignatureVerifier {
fn verify(message_hash: felt252, public_key: Span<felt252>, signature: Span<felt252>) -> bool {
if public_key.len() != PUBLIC_KEY_FELTS || signature.len() != SIGNATURE_FELTS {
return false;
}
let h_ntt = match packing::unpack_512_u16(public_key) {
Some(value) => value,
None => { return false; },
};
let s1 = match packing::unpack_512_u16(signature.slice(0, 29)) {
Some(value) => value,
None => { return false; },
};
let salt_a = *signature.at(29);
let salt_b = *signature.at(30);
let mul_hint = match packing::unpack_512_u16(signature.slice(31, 29)) {
Some(value) => value,
None => { return false; },
};
let message_point = match hash_to_point_shake_512(message_hash, salt_a, salt_b) {
Some(value) => value,
None => { return false; },
};
falcon::verify_512_with_hint_u16(
s1.span(), h_ntt.span(), mul_hint.span(), message_point.span(),
)
}

fn is_valid_public_key(public_key: Span<felt252>) -> bool {
if public_key.len() != PUBLIC_KEY_FELTS {
return false;
}
match packing::unpack_512_u16(public_key) {
Some(_) => true,
None => false,
}
}
}

/// Verifier for the 31-felt SHAKE-256 signature that recomputes the product on-chain.
pub(crate) impl Falcon512ShakeDirectVerifier of Falcon512SignatureVerifier {
fn verify(message_hash: felt252, public_key: Span<felt252>, signature: Span<felt252>) -> bool {
if public_key.len() != PUBLIC_KEY_FELTS || signature.len() != DIRECT_SIGNATURE_FELTS {
return false;
}
let h_ntt = match packing::unpack_512_u16(public_key) {
Some(value) => value,
None => { return false; },
};
let s1 = match packing::unpack_512_u16(signature.slice(0, 29)) {
Some(value) => value,
None => { return false; },
};
let salt_a = *signature.at(29);
let salt_b = *signature.at(30);
let message_point = match hash_to_point_shake_512(message_hash, salt_a, salt_b) {
Some(value) => value,
None => { return false; },
};
falcon::verify_512_direct_u16(s1.span(), h_ntt.span(), message_point.span())
}

fn is_valid_public_key(public_key: Span<felt252>) -> bool {
Falcon512ShakeVerifier::is_valid_public_key(public_key)
}
}
180 changes: 180 additions & 0 deletions packages/account/src/falcon_512/account.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts for Cairo v4.0.0-alpha.1 (account/src/falcon_512/account.cairo)

//! Shared account component for the Falcon-512 SHAKE account contracts.

/// Verification surface implemented by each Falcon-512 account variant.
pub(crate) trait Falcon512SignatureVerifier {
/// Returns whether `signature` authenticates `message_hash` under `public_key`.
fn verify(message_hash: felt252, public_key: Span<felt252>, signature: Span<felt252>) -> bool;

/// Returns whether the packed public key has the required canonical encoding.
fn is_valid_public_key(public_key: Span<felt252>) -> bool;
}

/// Account component for immutable, array-encoded Falcon-512 public keys.
#[starknet::component]
pub(crate) mod Falcon512AccountComponent {
use core::num::traits::Zero;
use openzeppelin_interfaces::accounts as interface;
use openzeppelin_introspection::src5::SRC5Component;
use openzeppelin_introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait;
use openzeppelin_utils::execution::execute_single_call;
use starknet::account::Call;
use starknet::storage::{MutableVecTrait, StoragePointerReadAccess, Vec, VecTrait};
use crate::utils::is_tx_version_valid;
use super::Falcon512SignatureVerifier;

#[storage]
pub struct Storage {
pub public_key: Vec<felt252>,
}

#[event]
#[derive(Drop, Debug, PartialEq, starknet::Event)]
pub enum Event {}

pub mod Errors {
pub const INVALID_CALLER: felt252 = 'Account: invalid caller';
pub const INVALID_PUBLIC_KEY: felt252 = 'Account: invalid public key';
pub const INVALID_SIGNATURE: felt252 = 'Account: invalid signature';
pub const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version';
}

#[embeddable_as(SRC6Impl)]
impl SRC6<
TContractState,
impl Verifier: Falcon512SignatureVerifier,
+HasComponent<TContractState>,
+SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of interface::ISRC6<ComponentState<TContractState>> {
/// Executes calls forwarded by the account after protocol validation succeeds.
fn __execute__(self: @ComponentState<TContractState>, calls: Array<Call>) {
let sender = starknet::get_caller_address();
assert(sender.is_zero(), Errors::INVALID_CALLER);
assert(is_tx_version_valid(), Errors::INVALID_TX_VERSION);

for call in calls.span() {
execute_single_call(call);
}
}

/// Validates an invoke transaction with the current transaction signature.
fn __validate__(self: @ComponentState<TContractState>, calls: Array<Call>) -> felt252 {
let _ = calls;
self.validate_transaction::<Verifier>()
}

/// Verifies a signature for an arbitrary message hash.
fn is_valid_signature(
self: @ComponentState<TContractState>, hash: felt252, signature: Array<felt252>,
) -> felt252 {
if Verifier::verify(hash, self.read_public_key().span(), signature.span()) {
starknet::VALIDATED
} else {
0
}
}
}

#[embeddable_as(DeclarerImpl)]
impl Declarer<
TContractState,
impl Verifier: Falcon512SignatureVerifier,
+HasComponent<TContractState>,
+SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of interface::IDeclarer<ComponentState<TContractState>> {
/// Validates a declare transaction with the current transaction signature.
fn __validate_declare__(
self: @ComponentState<TContractState>, class_hash: felt252,
) -> felt252 {
let _ = class_hash;
self.validate_transaction::<Verifier>()
}
}

#[embeddable_as(DeployableImpl)]
impl Deployable<
TContractState,
impl Verifier: Falcon512SignatureVerifier,
+HasComponent<TContractState>,
+SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of interface::IFeltArrayDeployable<ComponentState<TContractState>> {
/// Validates a deploy-account transaction with the current transaction signature.
fn __validate_deploy__(
self: @ComponentState<TContractState>,
class_hash: felt252,
contract_address_salt: felt252,
public_key: Array<felt252>,
) -> felt252 {
let _ = class_hash;
let _ = contract_address_salt;
let _ = public_key;
self.validate_transaction::<Verifier>()
}
}

#[embeddable_as(PublicKeyImpl)]
impl PublicKey<
TContractState,
+HasComponent<TContractState>,
+SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of interface::IFeltArrayPublicKey<ComponentState<TContractState>> {
/// Returns the immutable packed Falcon-512 public key.
fn get_public_key(self: @ComponentState<TContractState>) -> Array<felt252> {
self.read_public_key()
}
}

#[generate_trait]
pub impl InternalImpl<
TContractState,
+HasComponent<TContractState>,
impl SRC5: SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of InternalTrait<TContractState> {
/// Validates and stores the immutable packed public key and registers SRC6 support.
fn initializer<impl Verifier: Falcon512SignatureVerifier>(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Severity 2 — Low: the Verifier generic can silently disagree between initializer and the embedded impls.

Verifier is a function-level generic on InternalTrait but an impl-level generic on the embeddable impls — nothing forces them to agree, so a contract can embed Falcon512AccountMixinImpl<ContractState, VerifierA> while calling initializer::<VerifierB>. Harmless today (both supplied verifiers share is_valid_public_key), but a third strategy with a different key encoding would let initializer accept a key the runtime verifier rejects.

Suggested fix: move Verifier to an impl-level generic on InternalImpl.

Also: initializer is pub with no re-initialization guard (a second call overwrites the key and emits OwnerAdded with no OwnerRemoved). That mirrors AccountComponent/EthAccountComponent so it's consistency-preserving, but no test pins the behaviour.

ref self: ComponentState<TContractState>, public_key: Array<felt252>,
) {
assert(Verifier::is_valid_public_key(public_key.span()), Errors::INVALID_PUBLIC_KEY);

let mut src5_component = get_dep_component_mut!(ref self, SRC5);
src5_component.register_interface(interface::ISRC6_ID);

for felt in public_key {
self.public_key.push(felt);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Validates the transaction hash against the transaction signature.
fn validate_transaction<impl Verifier: Falcon512SignatureVerifier>(
self: @ComponentState<TContractState>,
) -> felt252 {
let tx_info = starknet::get_tx_info().unbox();
assert(
Verifier::verify(
tx_info.transaction_hash, self.read_public_key().span(), tx_info.signature,
),
Errors::INVALID_SIGNATURE,
);
starknet::VALIDATED
}

/// Reads the stored public key into its 29-felt packed representation.
fn read_public_key(self: @ComponentState<TContractState>) -> Array<felt252> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Severity 3 — Medium: Vec<felt252> storage doubles the syscall count on every operation.

VecImpl::at() re-reads the length per element (corelib 2.18: assert!(index < self.len()) where len() is a storage read), so read_public_key issues 59 StorageReads for 29 felts — verified against test syscall counts (207 = 30 deploy + 3×59). This is paid by every __validate__, __validate_declare__, __validate_deploy__, is_valid_signature, get_public_key, and every SRC9 outside execution. The constructor does 59 writes + 30 reads (28 of 29 length-writes pure overhead), and rotation costs 88 reads + 29 writes.

The length is fixed at 29 by construction (PUBLIC_KEY_FELTS; line 397 already asserts it never changes), so Vec buys nothing here.

Suggested fix: Map<u64, felt252> + the compile-time constant, or a #[derive(starknet::Store)] struct in consecutive slots (which also drops the per-element Poseidon path hash). 59 → 29 reads, ~0.75M gas per validation.

let mut public_key = array![];
let len = self.public_key.len();
let mut index = 0;
while index != len {
public_key.append(self.public_key.at(index).read());
index += 1;
}
Comment on lines +412 to +416

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for i in 0..len { ... }

public_key
}
}
}
Loading
Loading