Skip to content
Merged
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ pub(crate) mod RequestApprovalsComponent {
use starkware_utils::signature::stark::{
HashType, PublicKey, Signature, validate_stark_signature,
};
use starkware_utils::time::time::{Time, Timestamp};

#[storage]
pub struct Storage {
approved_requests: Map<HashType, RequestStatus>,
/// Stores forced action requests as (timestamp, status) by request_hash.
forced_action_requests: Map<HashType, (Timestamp, RequestStatus)>,
}

#[event]
Expand Down Expand Up @@ -48,17 +51,60 @@ pub(crate) mod RequestApprovalsComponent {
public_key: PublicKey,
signature: Signature,
args: T,
) -> HashType {
let request_hash = self.store_approval(:public_key, :args);
if let Option::Some(owner_account) = owner_account {
assert(owner_account == get_caller_address(), errors::CALLER_IS_NOT_OWNER_ACCOUNT);
}
validate_stark_signature(:public_key, msg_hash: request_hash, :signature);
request_hash
}
/// Registers an approval for a request without signature or caller validation.
///
/// This is intended for use with forced requests, where we track the same logical
/// request in two maps:
/// - `approved_requests` (generic approval status)
/// - `forced_action_requests` (forced metadata: timestamp + status)
/// This function is used to register the original request hash in the approvals map.

fn store_approval<T, +OffchainMessageHash<T>, +Drop<T>>(
ref self: ComponentState<TContractState>, public_key: PublicKey, args: T,
) -> HashType {
let request_hash = args.get_message_hash(:public_key);
assert(
self._get_request_status(:request_hash) == RequestStatus::NOT_REGISTERED,
errors::REQUEST_ALREADY_REGISTERED,
);
self.approved_requests.write(key: request_hash, value: RequestStatus::PENDING);
request_hash
}
/// Registers a forced approval action.
/// If the owner_account is non-zero, the caller must be the owner_account.
/// The request is signed with the public key.
/// The request is stored with a status of PENDING, and the current time.
fn register_forced_approval<T, +OffchainMessageHash<T>, +Drop<T>>(
ref self: ComponentState<TContractState>,
owner_account: Option<ContractAddress>,
public_key: PublicKey,
signature: Signature,
args: T,
) -> HashType {
let request_hash = args.get_message_hash(:public_key);

assert(
self
._get_forced_action_request_status(
request_hash,
) == RequestStatus::NOT_REGISTERED,
errors::REQUEST_ALREADY_REGISTERED,
);
if let Option::Some(owner_account) = owner_account {
assert(owner_account == get_caller_address(), errors::CALLER_IS_NOT_OWNER_ACCOUNT);
}
validate_stark_signature(:public_key, msg_hash: request_hash, :signature);
self.approved_requests.write(key: request_hash, value: RequestStatus::PENDING);
self
.forced_action_requests
.write(key: request_hash, value: (Time::now(), RequestStatus::PENDING));
Copy link

Choose a reason for hiding this comment

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

Bug: Forced approval misses required dual-map registration

The register_forced_approval function only writes to forced_action_requests but doesn't register the request in approved_requests. According to the documentation for store_approval, forced requests should be tracked in both maps to maintain consistency. This breaks the dual-map invariant and could cause issues when the request is later processed, as _get_request_status() will return NOT_REGISTERED instead of PENDING.

Fix in Cursor Fix in Web

request_hash
}

Expand All @@ -80,6 +126,30 @@ pub(crate) mod RequestApprovalsComponent {
self.approved_requests.write(request_hash, RequestStatus::PROCESSED);
request_hash
}

/// Consumes a forced-approved request and updates its status to DONE.
///
/// Validations:
/// - The request must be in a PENDING state.
///
/// Returns:
/// - The timestamp of when the forced request was approved.
/// - The hash of the forced request.
fn consume_forced_approved_request<T, +OffchainMessageHash<T>, +Drop<T>>(
ref self: ComponentState<TContractState>, args: T, public_key: PublicKey,
) -> (Timestamp, HashType) {
let request_hash = args.get_message_hash(:public_key);
let (request_time, request_status) = self.forced_action_requests.read(request_hash);
match request_status {
RequestStatus::NOT_REGISTERED => panic_with_felt252(errors::REQUEST_NOT_REGISTERED),
RequestStatus::PROCESSED => panic_with_felt252(errors::REQUEST_ALREADY_PROCESSED),
RequestStatus::PENDING => {},
}
self
.forced_action_requests
.write(request_hash, (request_time, RequestStatus::PROCESSED));
(request_time, request_hash)
}
}

#[generate_trait]
Expand All @@ -91,5 +161,11 @@ pub(crate) mod RequestApprovalsComponent {
) -> RequestStatus {
self.approved_requests.read(request_hash)
}
fn _get_forced_action_request_status(
self: @ComponentState<TContractState>, request_hash: HashType,
) -> RequestStatus {
let (_, status) = self.forced_action_requests.read(request_hash);
status
}
}
}