From 7d88b37f2158dd8d9998d1f467a7027e004c46a5 Mon Sep 17 00:00:00 2001 From: Brandon R <54774639+b-j-roberts@users.noreply.github.com> Date: Tue, 12 Mar 2024 09:11:36 -0500 Subject: [PATCH] dev: Comments and naming patches, typo fixes (#112) * Update comments, patched some names in the fee vault, indexed fee vault event keys, and typos * scarb fmt --- src/blobstreamx.cairo | 11 +++-- src/succinctx/fee_vault.cairo | 46 +++++++++++-------- .../function_registry/component.cairo | 6 ++- .../function_registry/interfaces.cairo | 4 +- src/succinctx/interfaces.cairo | 8 ++-- src/succinctx/tests/test_fee_vault.cairo | 20 ++++---- 6 files changed, 54 insertions(+), 41 deletions(-) diff --git a/src/blobstreamx.cairo b/src/blobstreamx.cairo index 983711c..847cfb0 100644 --- a/src/blobstreamx.cairo +++ b/src/blobstreamx.cairo @@ -199,7 +199,7 @@ mod blobstreamx { self.next_header_function_id.write(_function_id); } - /// Prove the validity of the header at the target block and a data commitment for the block range [latestBlock, _targetBlock). + /// Request a header_range proof for the next header hash and a data commitment for the block range [latest_block, _target_block). /// Used to skip from the latest block to the target block. /// /// # Arguments @@ -245,6 +245,7 @@ mod blobstreamx { } /// Commits the new header at targetBlock and the data commitment for the block range [trustedBlock, targetBlock). + /// This is called as a callback from the gateway after a request_header_range. /// /// # Arguments /// @@ -265,6 +266,7 @@ mod blobstreamx { input.append_u256(trusted_header); input.append_u64(_target_block); + // Get the output of the header_range proof from the gateway. let (target_header, data_commitment) = ISuccinctGatewayDispatcher { contract_address: self.get_gateway() } @@ -282,13 +284,14 @@ mod blobstreamx { end_block: _target_block, } ); - self.emit(HeadUpdate { target_block: latest_block, target_header: target_header }); + self.emit(HeadUpdate { target_block: _target_block, target_header: target_header }); self.state_proof_nonce.write(proof_nonce + 1); self.latest_block.write(_target_block); } - /// Prove the validity of the next header and a data commitment for the block range [latestBlock, latestBlock + 1). + /// Request a next_header proof for the next header hash and a data commitment for the block range [latest_block, latest_block + 1). + /// Rarely used, only if the validator set changes by more than 2/3 in a single block. fn request_next_header(ref self: ContractState) { let latest_block = self.get_latest_block(); let latest_header = self.block_height_to_header_hash.read(latest_block); @@ -321,6 +324,7 @@ mod blobstreamx { /// Stores the new header for _trustedBlock + 1 and the data commitment for the block range [_trustedBlock, _trustedBlock + 1). + /// This is called as a callback from the gateway after a request_next_header. /// /// # Arguments /// @@ -336,6 +340,7 @@ mod blobstreamx { input.append_u64(_trusted_block); input.append_u256(trusted_header); + // Get the output of the next_header proof from the gateway. let (next_header, data_commitment) = ISuccinctGatewayDispatcher { contract_address: self.gateway.read() } diff --git a/src/succinctx/fee_vault.cairo b/src/succinctx/fee_vault.cairo index b4ac01a..a9c8221 100644 --- a/src/succinctx/fee_vault.cairo +++ b/src/succinctx/fee_vault.cairo @@ -18,6 +18,7 @@ mod succinct_fee_vault { #[storage] struct Storage { + // balances[token][account] => token balance for Succinct services. balances: LegacyMap::<(ContractAddress, ContractAddress), u256>, allowed_deductors: LegacyMap::, native_currency_address: ContractAddress, @@ -42,20 +43,26 @@ mod succinct_fee_vault { #[derive(Drop, starknet::Event)] struct Received { + #[key] account: ContractAddress, + #[key] token: ContractAddress, amount: u256 } #[derive(Drop, starknet::Event)] struct Deducted { + #[key] account: ContractAddress, + #[key] token: ContractAddress, amount: u256 } #[derive(Drop, starknet::Event)] struct Collected { + #[key] to: ContractAddress, + #[key] token: ContractAddress, amount: u256, } @@ -89,7 +96,7 @@ mod succinct_fee_vault { #[abi(embed_v0)] impl IFeeVaultImpl of IFeeVault { /// Get the current native currency address - /// # Returns + /// # Returns /// The native currency address defined. fn get_native_currency(self: @ContractState) -> ContractAddress { self.native_currency_address.read() @@ -105,26 +112,15 @@ mod succinct_fee_vault { } - /// Get the deductor status + /// Check if the specified deductor is allowed to deduct from the vault. /// # Arguments - /// * `_deductor` - The deductor to retrieve the status. - /// # Returns - /// The boolean associated with the deductor status - fn get_deductor_status(self: @ContractState, _deductor: ContractAddress) -> bool { + /// * `_deductor` - The deductor to check. + /// # Returns + /// True if the deductor is allowed to deduct from the vault, false otherwise. + fn is_deductor(self: @ContractState, _deductor: ContractAddress) -> bool { self.allowed_deductors.read(_deductor) } - /// Get the balance for a given token and account - /// # Arguments - /// * `_account` - The account to retrieve the balance information. - /// * `_token` - The token address to consider. - /// # Returns - /// The associated balance. - fn get_balances_infos( - self: @ContractState, _account: ContractAddress, _token: ContractAddress - ) -> u256 { - self.balances.read((_token, _account)) - } /// Add the specified deductor /// # Arguments /// * `_deductor` - The address of the deductor to add. @@ -141,6 +137,18 @@ mod succinct_fee_vault { self.allowed_deductors.write(_deductor, false); } + /// Get the balance for a given token and account to use for Succinct services. + /// # Arguments + /// * `_account` - The account to retrieve the balance for. + /// * `_token` - The token address to consider. + /// # Returns + /// The associated balance. + fn get_account_balance( + self: @ContractState, _account: ContractAddress, _token: ContractAddress + ) -> u256 { + self.balances.read((_token, _account)) + } + /// Deposit the specified amount of native currency from the caller. /// Dev: the native currency address is defined in the storage slot native_currency /// Dev: MUST approve this contract to spend at least _amount of the native_currency before calling this. @@ -183,9 +191,7 @@ mod succinct_fee_vault { /// # Arguments /// * `_account` - The account to deduct the native currency from. fn deduct_native(ref self: ContractState, _account: ContractAddress) { - let caller_address = get_caller_address(); let native_currency = self.native_currency_address.read(); - assert(self.allowed_deductors.read(caller_address), Errors::OnlyDeductor); self .deduct( _account, native_currency, starknet::info::get_tx_info().unbox().max_fee.into() @@ -204,7 +210,7 @@ mod succinct_fee_vault { _amount: u256 ) { let caller_address = get_caller_address(); - assert(self.allowed_deductors.read(caller_address), Errors::OnlyDeductor); + assert(self.is_deductor(caller_address), Errors::OnlyDeductor); assert(!_account.is_zero(), Errors::InvalidAccount); assert(!_token.is_zero(), Errors::InvalidToken); let current_balance = self.balances.read((_token, _account)); diff --git a/src/succinctx/function_registry/component.cairo b/src/succinctx/function_registry/component.cairo index 47ef9df..b5b56b9 100644 --- a/src/succinctx/function_registry/component.cairo +++ b/src/succinctx/function_registry/component.cairo @@ -49,10 +49,12 @@ mod function_registry_cpt { impl FunctionRegistry< TContractState, +HasComponent > of IFunctionRegistry> { - fn verifiers(self: @ComponentState, function_id: u256) -> ContractAddress { + fn get_verifier( + self: @ComponentState, function_id: u256 + ) -> ContractAddress { self.verifiers.read(function_id) } - fn verifier_owners( + fn get_verifier_owner( self: @ComponentState, function_id: u256 ) -> ContractAddress { self.verifier_owners.read(function_id) diff --git a/src/succinctx/function_registry/interfaces.cairo b/src/succinctx/function_registry/interfaces.cairo index c64fdba..bf626a2 100644 --- a/src/succinctx/function_registry/interfaces.cairo +++ b/src/succinctx/function_registry/interfaces.cairo @@ -2,8 +2,8 @@ use starknet::ContractAddress; #[starknet::interface] trait IFunctionRegistry { - fn verifiers(self: @TContractState, function_id: u256) -> ContractAddress; - fn verifier_owners(self: @TContractState, function_id: u256) -> ContractAddress; + fn get_verifier(self: @TContractState, function_id: u256) -> ContractAddress; + fn get_verifier_owner(self: @TContractState, function_id: u256) -> ContractAddress; fn get_function_id(self: @TContractState, owner: ContractAddress, name: felt252) -> u256; fn register_function( ref self: TContractState, owner: ContractAddress, verifier: ContractAddress, name: felt252 diff --git a/src/succinctx/interfaces.cairo b/src/succinctx/interfaces.cairo index 1b58ad3..dc1c066 100644 --- a/src/succinctx/interfaces.cairo +++ b/src/succinctx/interfaces.cairo @@ -56,12 +56,12 @@ trait ISuccinctGateway { trait IFeeVault { fn get_native_currency(self: @TContractState) -> ContractAddress; fn set_native_currency(ref self: TContractState, _new_native_address: ContractAddress); - fn get_deductor_status(self: @TContractState, _deductor: ContractAddress) -> bool; - fn get_balances_infos( - self: @TContractState, _account: ContractAddress, _token: ContractAddress - ) -> u256; + fn is_deductor(self: @TContractState, _deductor: ContractAddress) -> bool; fn add_deductor(ref self: TContractState, _deductor: ContractAddress); fn remove_deductor(ref self: TContractState, _deductor: ContractAddress); + fn get_account_balance( + self: @TContractState, _account: ContractAddress, _token: ContractAddress + ) -> u256; fn deposit_native(ref self: TContractState, _account: ContractAddress); fn deposit( ref self: TContractState, _account: ContractAddress, _token: ContractAddress, _amount: u256 diff --git a/src/succinctx/tests/test_fee_vault.cairo b/src/succinctx/tests/test_fee_vault.cairo index 66647df..a5d95bb 100644 --- a/src/succinctx/tests/test_fee_vault.cairo +++ b/src/succinctx/tests/test_fee_vault.cairo @@ -63,11 +63,11 @@ fn fee_vault_deductor_operations() { start_prank(CheatTarget::One(fee_vault.contract_address), OWNER()); // Adding a new deductor fee_vault.add_deductor(SPENDER()); - assert(fee_vault.get_deductor_status(SPENDER()), 'deductor status not updated'); + assert(fee_vault.is_deductor(SPENDER()), 'deductors not updated'); // Removing the same deductor fee_vault.remove_deductor(SPENDER()); - assert(!fee_vault.get_deductor_status(SPENDER()), 'deductor status not updated'); + assert(!fee_vault.is_deductor(SPENDER()), 'deductors not updated'); stop_prank(CheatTarget::One(fee_vault.contract_address)); } @@ -78,7 +78,7 @@ fn fee_vault_add_deductor_fails_if_not_owner() { let (_, fee_vault) = setup_contracts(); // Adding a new deductor fee_vault.add_deductor(SPENDER()); - assert(fee_vault.get_deductor_status(SPENDER()), 'deductor status not updated'); + assert(fee_vault.is_deductor(SPENDER()), 'deductors not updated'); } @@ -88,7 +88,7 @@ fn fee_vault_remove_deductor_fails_if_not_owner() { let (_, fee_vault) = setup_contracts(); // Adding a new deductor fee_vault.remove_deductor(SPENDER()); - assert(!fee_vault.get_deductor_status(SPENDER()), 'deductor status not updated'); + assert(!fee_vault.is_deductor(SPENDER()), 'deductors not updated'); } #[test] @@ -102,7 +102,7 @@ fn fee_vault_deposit_native() { start_prank(CheatTarget::One(fee_vault.contract_address), SPENDER()); fee_vault.deposit_native(SPENDER()); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == fee, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == fee, 'balances not updated' ); stop_prank(CheatTarget::One(fee_vault.contract_address)); @@ -118,7 +118,7 @@ fn fee_vault_deposit() { start_prank(CheatTarget::One(fee_vault.contract_address), SPENDER()); fee_vault.deposit(SPENDER(), erc20.contract_address, 0x10000); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == 0x10000, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == 0x10000, 'balances deposit not updated' ); stop_prank(CheatTarget::One(fee_vault.contract_address)); @@ -146,7 +146,7 @@ fn fee_vault_deposit_fails_if_insufficent_allowance() { start_prank(CheatTarget::One(fee_vault.contract_address), SPENDER()); fee_vault.deposit(SPENDER(), erc20.contract_address, 0x10000); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == 0x10000, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == 0x10000, 'balances deposit not updated' ); stop_prank(CheatTarget::One(fee_vault.contract_address)); @@ -166,12 +166,12 @@ fn fee_vault_deduct_native() { start_prank(CheatTarget::One(fee_vault.contract_address), SPENDER()); fee_vault.deposit_native(SPENDER()); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == fee, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == fee, 'balances deposit not updated' ); fee_vault.deduct_native(SPENDER()); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == 0, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == 0, 'balances deduct not updated' ); stop_prank(CheatTarget::One(fee_vault.contract_address)); @@ -191,7 +191,7 @@ fn fee_vault_deduct() { fee_vault.deposit(SPENDER(), erc20.contract_address, 0x10000); fee_vault.deduct(SPENDER(), erc20.contract_address, 0x8000); assert( - fee_vault.get_balances_infos(SPENDER(), erc20.contract_address) == 0x10000 - 0x8000, + fee_vault.get_account_balance(SPENDER(), erc20.contract_address) == 0x10000 - 0x8000, 'balances deduct not updated' ); stop_prank(CheatTarget::One(fee_vault.contract_address));