Skip to content

Commit

Permalink
fix: unhandled cases where contract is not found (#65)
Browse files Browse the repository at this point in the history
* fix: unhandled cases where contract is not found

When the path to the contract given in `Scarb.toml` does not contain any
contracts, the verification continues without throwing an error. This is
problematic and the verifier should throw an clear error indicating
this.

* fix: issues with contracts path checks
  • Loading branch information
cwkang1998 authored Aug 22, 2024
1 parent 63a84c8 commit 67a5148
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 3 deletions.
28 changes: 26 additions & 2 deletions crates/voyager-resolver-cairo/src/compiler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use anyhow::{Context, Result};
use anyhow::{anyhow, Context, Result};
use cairo_lang_compiler::db::RootDatabase;
use cairo_lang_defs::ids::ModuleId;
use cairo_lang_filesystem::db::FilesGroup;
use cairo_lang_filesystem::ids::{CrateId, CrateLongId, FileLongId};
use camino::Utf8PathBuf;
use std::collections::HashMap;
use std::path::PathBuf;
use std::path::{Path, PathBuf};

use cairo_lang_defs::db::DefsGroup;
use cairo_lang_diagnostics::ToOption;
Expand Down Expand Up @@ -99,6 +99,10 @@ impl Compiler for VoyagerGenerator {
// This updates the compiler database with the crate roots of the external dependencies.
// This enables resolving external dependencies paths.
let manifest_path: PathBuf = unit.main_component().package.manifest_path().into();
let root_path = match manifest_path.parent() {
Some(path) => path,
None => Path::new(""),
};
let metadata = read_scarb_metadata(&manifest_path)
.expect("Failed to obtain scarb metadata from manifest file.");
update_crate_roots_from_metadata(db, metadata.clone());
Expand Down Expand Up @@ -127,8 +131,27 @@ impl Compiler for VoyagerGenerator {

// Read Scarb manifest file to get the list of contracts to verify from the [tool.voyager] section.
// This returns the relative file paths of the contracts to verify.
// TODO: handle when this is empty.
let contracts_to_verify = get_contracts_to_verify(&unit.main_component().package)?;

if contracts_to_verify.is_empty() {
return Err(anyhow!("No contracts found."));
}
if contracts_to_verify.len() > 1 {
return Err(anyhow!("Currently doesn't support multiple contracts."));
}

// Check if the path exists, which requires us to know the absolute path.
if !root_path
.join("src")
.join(contracts_to_verify[0].clone())
.exists()
{
return Err(anyhow!(
"Unable to find the contract file given in Scarb.toml"
));
}

// Collect the CairoModule corresponding to the file paths of the contracts.
let modules_to_verify = project_modules
.iter()
Expand Down Expand Up @@ -233,6 +256,7 @@ impl VoyagerGenerator {
// Extract a vector of modules for the crate.
// We only collect "file" modules, which are related to a Cairo file.
// Internal modules are not collected here.
// TODO: we should also collect internal modules, since they matter!
let crate_modules = collect_crate_module_files(db, crate_id)?;
Ok(CairoCrate {
root_dir: crate_root_dir,
Expand Down
42 changes: 41 additions & 1 deletion crates/voyager-resolver-cairo/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::any::Any;
use std::env;

use anyhow::Result;
use anyhow::{anyhow, Result};
use scarb::compiler::CompilerRepository;
use scarb::core::{Config, TargetKind};
use scarb::ops;
Expand All @@ -11,6 +12,45 @@ use voyager_resolver_cairo::compiler::scarb_utils::get_contracts_to_verify;
use voyager_resolver_cairo::compiler::VoyagerGenerator;
use voyager_resolver_cairo::utils::run_scarb_build;

#[test]
fn test_incorrect_contract_path_given() -> Result<()> {
let mut compilers = CompilerRepository::empty();
compilers.add(Box::new(VoyagerGenerator)).unwrap();
let source_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("tests/test_data")
.join("project_w_incorrect_contract_path");
let manifest_path = source_dir.join("Scarb.toml");

let config = Config::builder(manifest_path.to_str().unwrap())
.ui_verbosity(Verbosity::Verbose)
.log_filter_directive(env::var_os("SCARB_LOG"))
.compilers(compilers)
.build()
.unwrap();

let ws = ops::read_workspace(config.manifest_path(), &config).unwrap_or_else(|err| {
eprintln!("error: {}", err);
std::process::exit(1);
});
let package_ids = ws.members().map(|p| p.id.clone()).collect();
let compile_opts = ops::CompileOpts {
include_targets: vec![TargetKind::STARKNET_CONTRACT],
exclude_targets: vec![],
};

let result = ops::compile(package_ids, compile_opts, &ws);
assert!(result.is_err());

let reduced_project_path = source_dir.join("voyager-verify/project_w_incorrect_contract_path");
println!(
"Reduced project path: {}",
reduced_project_path.to_str().unwrap()
);
let compile_result = run_scarb_build(reduced_project_path.to_str().unwrap());
assert!(compile_result.is_err());
Ok(())
}

#[test]
fn test_get_contracts_to_verify() {
let mut compilers = CompilerRepository::empty();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
target
voyager-verify
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Code generated by scarb DO NOT EDIT.
version = 1

[[package]]
name = "project_w_incorrect_contract_path"
version = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "project_w_incorrect_contract_path"
version = "0.1.0"

# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest

[dependencies]
starknet = "=2.4.3"

[[target.starknet-contract]]

[tool.voyager]
# Explicitly incorrect
ERC20 = { path = "ERC20.cairo", address = "0x12345" }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mod ERC20;
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
#[starknet::contract]
mod ERC20 {
use zeroable::Zeroable;
use starknet::get_caller_address;
use starknet::contract_address_const;
use starknet::ContractAddress;

#[storage]
struct Storage {
name: felt252,
symbol: felt252,
decimals: u8,
total_supply: u256,
balances: LegacyMap::<ContractAddress, u256>,
allowances: LegacyMap::<(ContractAddress, ContractAddress), u256>,
}

#[event]
#[derive(Drop, starknet::Event)]
enum Event {
Approval: Approval,
Transfer: Transfer,
}

#[derive(Drop, starknet::Event)]
struct Transfer {
from: ContractAddress,
to: ContractAddress,
value: u256
}

#[derive(Drop, starknet::Event)]
struct Approval {
owner: ContractAddress,
spender: ContractAddress,
value: u256
}

#[constructor]
fn constructor(
ref self: ContractState,
name_: felt252,
symbol_: felt252,
decimals_: u8,
initial_supply: u256,
recipient: ContractAddress
) {
self.name.write(name_);
self.symbol.write(symbol_);
self.decimals.write(decimals_);
assert(!recipient.is_zero(), 'ERC20: mint to the 0 address');
self.total_supply.write(initial_supply);
self.balances.write(recipient, initial_supply);
self
.emit(
Transfer {
from: contract_address_const::<0>(), to: recipient, value: initial_supply
}
);
}

#[external(v0)]
fn get_name(self: @ContractState) -> felt252 {
self.name.read()
}

#[external(v0)]
fn get_symbol(self: @ContractState) -> felt252 {
self.symbol.read()
}

#[external(v0)]
fn get_decimals(self: @ContractState) -> u8 {
self.decimals.read()
}

#[external(v0)]
fn get_total_supply(self: @ContractState) -> u256 {
self.total_supply.read()
}

#[external(v0)]
fn balance_of(self: @ContractState, account: ContractAddress) -> u256 {
self.balances.read(account)
}

#[external(v0)]
fn allowance(self: @ContractState, owner: ContractAddress, spender: ContractAddress) -> u256 {
self.allowances.read((owner, spender))
}

#[external(v0)]
fn transfer(ref self: ContractState, recipient: ContractAddress, amount: u256) {
let sender = get_caller_address();
transfer_helper(ref self, sender, recipient, amount);
}

#[external(v0)]
fn transfer_from(
ref self: ContractState, sender: ContractAddress, recipient: ContractAddress, amount: u256
) {
let caller = get_caller_address();
spend_allowance(ref self, sender, caller, amount);
transfer_helper(ref self, sender, recipient, amount);
}

#[external(v0)]
fn approve(ref self: ContractState, spender: ContractAddress, amount: u256) {
let caller = get_caller_address();
approve_helper(ref self, caller, spender, amount);
}

#[external(v0)]
fn increase_allowance(ref self: ContractState, spender: ContractAddress, added_value: u256) {
let caller = get_caller_address();
approve_helper(
ref self, caller, spender, self.allowances.read((caller, spender)) + added_value
);
}

#[external(v0)]
fn decrease_allowance(
ref self: ContractState, spender: ContractAddress, subtracted_value: u256
) {
let caller = get_caller_address();
approve_helper(
ref self, caller, spender, self.allowances.read((caller, spender)) - subtracted_value
);
}

fn transfer_helper(
ref self: ContractState, sender: ContractAddress, recipient: ContractAddress, amount: u256
) {
assert(!sender.is_zero(), 'ERC20: transfer from 0');
assert(!recipient.is_zero(), 'ERC20: transfer to 0');
self.balances.write(sender, self.balances.read(sender) - amount);
self.balances.write(recipient, self.balances.read(recipient) + amount);
self.emit(Transfer { from: sender, to: recipient, value: amount });
}

fn spend_allowance(
ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256
) {
let current_allowance = self.allowances.read((owner, spender));
let ONES_MASK = 0xffffffffffffffffffffffffffffffff_u128;
let is_unlimited_allowance = current_allowance.low == ONES_MASK
&& current_allowance.high == ONES_MASK;
if !is_unlimited_allowance {
approve_helper(ref self, owner, spender, current_allowance - amount);
}
}

fn approve_helper(
ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256
) {
assert(!spender.is_zero(), 'ERC20: approve from 0');
self.allowances.write((owner, spender), amount);
self.emit(Approval { owner, spender, value: amount });
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mod contracts;

0 comments on commit 67a5148

Please sign in to comment.