From e1324bc20186379bc39fd364229ff9feb5013bd5 Mon Sep 17 00:00:00 2001 From: Lucas Steuernagel Date: Thu, 14 Sep 2023 19:34:43 -0300 Subject: [PATCH] Verify account space specified in annotation Signed-off-by: Lucas Steuernagel --- src/bin/languageserver/mod.rs | 38 ++--- src/codegen/mod.rs | 19 +++ src/codegen/revert.rs | 2 +- src/codegen/solana_deploy.rs | 132 ++++++++++------- src/sema/ast.rs | 39 ++--- src/sema/dotgraphviz.rs | 49 +++---- src/sema/eval.rs | 28 ++-- src/sema/function_annotation.rs | 136 ++++++++---------- .../solana/annotations/invalid_space.sol | 14 ++ .../solana/annotations/not_enough_space.sol | 16 +++ .../solana/annotations/not_enough_space_2.sol | 14 ++ tests/solana_tests/create_contract.rs | 72 ++++++++++ 12 files changed, 353 insertions(+), 206 deletions(-) create mode 100644 tests/contract_testcases/solana/annotations/invalid_space.sol create mode 100644 tests/contract_testcases/solana/annotations/not_enough_space.sol create mode 100644 tests/contract_testcases/solana/annotations/not_enough_space_2.sol diff --git a/src/bin/languageserver/mod.rs b/src/bin/languageserver/mod.rs index 34de2e3e84..491cc3b036 100644 --- a/src/bin/languageserver/mod.rs +++ b/src/bin/languageserver/mod.rs @@ -1408,25 +1408,27 @@ impl<'a> Builder<'a> { continue; } - for note in &func.annotations { - match note { - ast::ConstructorAnnotation::Bump(expr) - | ast::ConstructorAnnotation::Seed(expr) - | ast::ConstructorAnnotation::Space(expr) => { - builder.expression(expr, &func.symtable) - } + if let Some(bump) = &func.annotations.bump { + builder.expression(&bump.1, &func.symtable); + } - ast::ConstructorAnnotation::Payer(loc, name) => { - builder.hovers.push(( - loc.file_no(), - HoverEntry { - start: loc.start(), - stop: loc.exclusive_end(), - val: format!("payer account: {name}"), - }, - )); - } - } + for seed in &func.annotations.seeds { + builder.expression(&seed.1, &func.symtable); + } + + if let Some(space) = &func.annotations.space { + builder.expression(&space.1, &func.symtable); + } + + if let Some((loc, name)) = &func.annotations.payer { + builder.hovers.push(( + loc.file_no(), + HoverEntry { + start: loc.start(), + stop: loc.exclusive_end(), + val: format!("payer account: {name}"), + }, + )); } for (i, param) in func.params.iter().enumerate() { diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 047f79002d..9da63ed993 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -42,12 +42,15 @@ use std::cmp::Ordering; use crate::codegen::cfg::ASTFunction; use crate::codegen::solana_accounts::account_management::manage_contract_accounts; use crate::codegen::yul::generate_yul_function_cfg; +use crate::sema::diagnostics::Diagnostics; +use crate::sema::eval::eval_const_number; use crate::sema::Recurse; #[cfg(feature = "wasm_opt")] use contract_build::OptimizationPasses; use num_bigint::{BigInt, Sign}; use num_rational::BigRational; use num_traits::{FromPrimitive, Zero}; +use solang_parser::diagnostics::Diagnostic; use solang_parser::{pt, pt::CodeLocation}; // The sizeof(struct account_data_header) @@ -330,6 +333,22 @@ fn layout(contract_no: usize, ns: &mut Namespace) { } } + let constructors = ns.contracts[contract_no].constructors(ns); + if !constructors.is_empty() { + if let Some((_, exp)) = &ns.functions[constructors[0]].annotations.space { + // This code path is only reachable on Solana + assert_eq!(ns.target, Target::Solana); + if let Ok((_, value)) = eval_const_number(exp, ns, &mut Diagnostics::default()) { + if slot > value { + ns.diagnostics.push(Diagnostic::error( + exp.loc(), + format!("contract requires at least {} bytes of space", slot), + )); + } + } + } + } + ns.contracts[contract_no].fixed_layout_size = slot; } diff --git a/src/codegen/revert.rs b/src/codegen/revert.rs index 47a047580b..b436346fa5 100644 --- a/src/codegen/revert.rs +++ b/src/codegen/revert.rs @@ -405,7 +405,7 @@ pub(crate) fn error_msg_with_loc(ns: &Namespace, error: String, loc: Option } } -fn string_to_expr(string: String) -> Expression { +pub(super) fn string_to_expr(string: String) -> Expression { Expression::FormatString { loc: Loc::Codegen, args: vec![( diff --git a/src/codegen/solana_deploy.rs b/src/codegen/solana_deploy.rs index 5db682a5f6..3224d758e8 100644 --- a/src/codegen/solana_deploy.rs +++ b/src/codegen/solana_deploy.rs @@ -4,13 +4,15 @@ use super::{ cfg::ReturnCode, expression, Builtin, ControlFlowGraph, Expression, Instr, Options, Type, Vartable, }; +use crate::codegen::revert::string_to_expr; use crate::codegen::solana_accounts::account_management::{ account_meta_literal, retrieve_key_from_account_info, }; use crate::sema::ast::{ - self, ArrayLength, CallTy, ConstructorAnnotation, Function, FunctionAttributes, Namespace, - StructType, + self, ArrayLength, CallTy, Function, FunctionAttributes, Namespace, StructType, }; +use crate::sema::diagnostics::Diagnostics; +use crate::sema::eval::eval_const_number; use crate::sema::solana_accounts::BuiltinAccounts; use base58::ToBase58; use num_bigint::{BigInt, Sign}; @@ -78,20 +80,12 @@ pub(super) fn solana_deploy( cfg.set_basic_block(id_fail); - let message = format!("program_id should be {}", program_id.to_base58()).into_bytes(); - - let expr = Expression::AllocDynamicBytes { - loc: Loc::Codegen, - ty: Type::String, - size: Box::new(Expression::NumberLiteral { - loc: Loc::Codegen, - ty: Type::Uint(32), - value: BigInt::from(message.len()), - }), - initializer: Some(message), - }; - - cfg.add(vartab, Instr::Print { expr }); + cfg.add( + vartab, + Instr::Print { + expr: string_to_expr(format!("program_id should be {}", program_id.to_base58())), + }, + ); cfg.add( vartab, @@ -249,11 +243,7 @@ pub(super) fn solana_deploy( } } - if let Some(ConstructorAnnotation::Payer(_, name)) = func - .annotations - .iter() - .find(|tag| matches!(tag, ConstructorAnnotation::Payer(..))) - { + if let Some((_, name)) = &func.annotations.payer { let metas_ty = Type::Array( Box::new(Type::Struct(StructType::AccountMeta)), vec![ArrayLength::Fixed(BigInt::from(2))], @@ -314,13 +304,57 @@ pub(super) fn solana_deploy( ); // Calculate minimum balance for rent-exempt - let (space, lamports) = if let Some(ConstructorAnnotation::Space(space_expr)) = func - .annotations - .iter() - .find(|tag| matches!(tag, ConstructorAnnotation::Space(..))) - { - let space_var = vartab.temp_name("space", &Type::Uint(64)); + let (space, lamports) = if let Some((_, space_expr)) = &func.annotations.space { let expr = expression(space_expr, cfg, contract_no, None, ns, vartab, opt); + // If the space is not a literal or a constant expression, + // we must verify if we are allocating enough space during runtime. + if eval_const_number(space_expr, ns, &mut Diagnostics::default()).is_err() { + let cond = Expression::MoreEqual { + loc: Loc::Codegen, + signed: false, + left: Box::new(expr.clone()), + right: Box::new(Expression::NumberLiteral { + loc: Loc::Codegen, + ty: Type::Uint(64), + value: contract.fixed_layout_size.clone(), + }), + }; + + let enough = cfg.new_basic_block("enough_space".to_string()); + let not_enough = cfg.new_basic_block("not_enough_space".to_string()); + + cfg.add( + vartab, + Instr::BranchCond { + cond, + true_block: enough, + false_block: not_enough, + }, + ); + + cfg.set_basic_block(not_enough); + cfg.add( + vartab, + Instr::Print { + expr: string_to_expr(format!( + "value passed for space is \ + insufficient. Contract requires at least {} bytes", + contract.fixed_layout_size + )), + }, + ); + + cfg.add( + vartab, + Instr::ReturnCode { + code: ReturnCode::AccountDataTooSmall, + }, + ); + + cfg.set_basic_block(enough); + } + + let space_var = vartab.temp_name("space", &Type::Uint(64)); cfg.add( vartab, @@ -494,34 +528,26 @@ pub(super) fn solana_deploy( ); // seeds - let mut seeds = Vec::new(); - let mut declared_bump = None; + let mut seeds = func + .annotations + .seeds + .iter() + .map(|seed| expression(&seed.1, cfg, contract_no, None, ns, vartab, opt)) + .collect::>(); - for note in &func.annotations { - match note { - ConstructorAnnotation::Seed(seed) => { - seeds.push(expression(seed, cfg, contract_no, None, ns, vartab, opt)); - } - ConstructorAnnotation::Bump(bump) => { - let expr = ast::Expression::Cast { - loc: Loc::Codegen, - to: Type::Slice(Type::Bytes(1).into()), - expr: ast::Expression::BytesCast { - loc: Loc::Codegen, - to: Type::DynamicBytes, - from: Type::Bytes(1), - expr: bump.clone().into(), - } - .into(), - }; - declared_bump = Some(expr); + if let Some((_, bump)) = &func.annotations.bump { + let expr = ast::Expression::Cast { + loc: Loc::Codegen, + to: Type::Slice(Type::Bytes(1).into()), + expr: ast::Expression::BytesCast { + loc: Loc::Codegen, + to: Type::DynamicBytes, + from: Type::Bytes(1), + expr: bump.clone().into(), } - _ => (), - } - } - - if let Some(bump) = declared_bump { - seeds.push(expression(&bump, cfg, contract_no, None, ns, vartab, opt)); + .into(), + }; + seeds.push(expression(&expr, cfg, contract_no, None, ns, vartab, opt)); } let seeds = if !seeds.is_empty() { diff --git a/src/sema/ast.rs b/src/sema/ast.rs index fe26c21f04..604e2802dc 100644 --- a/src/sema/ast.rs +++ b/src/sema/ast.rs @@ -347,7 +347,7 @@ pub struct Function { /// For overloaded functions this is the mangled (unique) name. pub mangled_name: String, /// Solana constructors may have seeds specified using @seed tags - pub annotations: Vec, + pub annotations: ConstructorAnnotations, /// Which contracts should we use the mangled name in? pub mangled_name_contracts: HashSet, /// This indexmap stores the accounts this functions needs to be called on Solana @@ -366,26 +366,17 @@ pub struct SolanaAccount { pub generated: bool, } -#[derive(Debug)] -pub enum ConstructorAnnotation { - Seed(Expression), - Payer(pt::Loc, String), - Space(Expression), - Bump(Expression), +#[derive(Debug, Default)] +pub struct ConstructorAnnotations { + // (annotation location, annotation expression) + pub seeds: Vec<(pt::Loc, Expression)>, + pub space: Option<(pt::Loc, Expression)>, + pub bump: Option<(pt::Loc, Expression)>, + // (annotation location, account name) + pub payer: Option<(pt::Loc, String)>, } -impl CodeLocation for ConstructorAnnotation { - fn loc(&self) -> pt::Loc { - match self { - ConstructorAnnotation::Seed(expr) - | ConstructorAnnotation::Space(expr) - | ConstructorAnnotation::Bump(expr) => expr.loc(), - ConstructorAnnotation::Payer(loc, _) => *loc, - } - } -} - -/// This trait provides a single interface for fetching paramenters, returns and the symbol table +/// This trait provides a single interface for fetching parameters, returns and the symbol table /// for both yul and solidity functions pub trait FunctionAttributes { fn get_symbol_table(&self) -> &Symtable; @@ -464,7 +455,7 @@ impl Function { symtable: Symtable::new(), emits_events: Vec::new(), mangled_name, - annotations: Vec::new(), + annotations: ConstructorAnnotations::default(), mangled_name_contracts: HashSet::new(), solana_accounts: IndexMap::new().into(), } @@ -509,16 +500,12 @@ impl Function { /// Does this function have an @payer annotation? pub fn has_payer_annotation(&self) -> bool { - self.annotations - .iter() - .any(|note| matches!(note, ConstructorAnnotation::Payer(..))) + self.annotations.payer.is_some() } /// Does this function have an @seed annotation? pub fn has_seed_annotation(&self) -> bool { - self.annotations - .iter() - .any(|note| matches!(note, ConstructorAnnotation::Seed(..))) + !self.annotations.seeds.is_empty() } /// Does this function have the pure state diff --git a/src/sema/dotgraphviz.rs b/src/sema/dotgraphviz.rs index a923107158..4871811b3d 100644 --- a/src/sema/dotgraphviz.rs +++ b/src/sema/dotgraphviz.rs @@ -214,33 +214,30 @@ impl Dot { } // Annotations - if !func.annotations.is_empty() { - let node = self.add_node( - Node::new("annotations", vec!["annotations".into()]), - Some(func_node), - Some(String::from("annotations")), - ); + let node = self.add_node( + Node::new("annotations", vec!["annotations".into()]), + Some(func_node), + Some(String::from("annotations")), + ); - for note in &func.annotations { - match note { - ConstructorAnnotation::Seed(expr) => { - self.add_expression(expr, Some(func), ns, node, "seed".into()); - } - ConstructorAnnotation::Space(expr) => { - self.add_expression(expr, Some(func), ns, node, "space".into()); - } - ConstructorAnnotation::Bump(expr) => { - self.add_expression(expr, Some(func), ns, node, "bump".into()); - } - ConstructorAnnotation::Payer(_, name) => { - self.add_node( - Node::new("payer", vec![name.clone()]), - Some(node), - Some(String::from("payer declaration")), - ); - } - }; - } + for seed in &func.annotations.seeds { + self.add_expression(&seed.1, Some(func), ns, node, "seed".into()); + } + + if let Some(space) = &func.annotations.space { + self.add_expression(&space.1, Some(func), ns, node, "space".into()); + } + + if let Some(bump) = &func.annotations.bump { + self.add_expression(&bump.1, Some(func), ns, node, "bump".into()); + } + + if let Some((_, name)) = &func.annotations.payer { + self.add_node( + Node::new("payer", vec![name.clone()]), + Some(node), + Some(String::from("payer declaration")), + ); } // bases diff --git a/src/sema/eval.rs b/src/sema/eval.rs index 7c851814d2..644f63f5f1 100644 --- a/src/sema/eval.rs +++ b/src/sema/eval.rs @@ -14,12 +14,22 @@ use solang_parser::pt; use solang_parser::pt::{CodeLocation, Loc}; use std::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Shl, Shr, Sub}; +/// This enum specifies the error `eval_const_number` is returning +pub enum EvaluationError { + NotAConstant, + MathError, +} + +impl From for () { + fn from(_value: EvaluationError) -> Self {} +} + /// Resolve an expression where a compile-time constant is expected pub fn eval_const_number( expr: &Expression, ns: &Namespace, diagnostics: &mut Diagnostics, -) -> Result<(pt::Loc, BigInt), ()> { +) -> Result<(pt::Loc, BigInt), EvaluationError> { match expr { Expression::Add { loc, left, right, .. @@ -50,7 +60,7 @@ pub fn eval_const_number( if divisor.is_zero() { diagnostics.push(Diagnostic::error(*loc, "divide by zero".to_string())); - Err(()) + Err(EvaluationError::MathError) } else { Ok((*loc, eval_const_number(left, ns, diagnostics)?.1 / divisor)) } @@ -63,7 +73,7 @@ pub fn eval_const_number( if divisor.is_zero() { diagnostics.push(Diagnostic::error(*loc, "divide by zero".to_string())); - Err(()) + Err(EvaluationError::MathError) } else { Ok((*loc, eval_const_number(left, ns, diagnostics)?.1 % divisor)) } @@ -99,7 +109,7 @@ pub fn eval_const_number( "power cannot take negative number as exponent".to_string(), )); - Err(()) + Err(EvaluationError::MathError) } else if e.sign() == Sign::NoSign { Ok((*loc, BigInt::one())) } else { @@ -122,7 +132,7 @@ pub fn eval_const_number( None => { diagnostics.push(Diagnostic::error(*loc, format!("cannot left shift by {r}"))); - return Err(()); + return Err(EvaluationError::MathError); } }; Ok((*loc, l << r)) @@ -137,7 +147,7 @@ pub fn eval_const_number( None => { diagnostics.push(Diagnostic::error(*loc, format!("right left shift by {r}"))); - return Err(()); + return Err(EvaluationError::MathError); } }; Ok((*loc, l >> r)) @@ -170,7 +180,7 @@ pub fn eval_const_number( eval_const_number(init, ns, diagnostics) } else { // we should have errored about this already - Err(()) + Err(EvaluationError::NotAConstant) } } Expression::ConstantVariable { @@ -184,7 +194,7 @@ pub fn eval_const_number( eval_const_number(init, ns, diagnostics) } else { // we should have errored about this already - Err(()) + Err(EvaluationError::NotAConstant) } } _ => { @@ -193,7 +203,7 @@ pub fn eval_const_number( "expression not allowed in constant number expression".to_string(), )); - Err(()) + Err(EvaluationError::NotAConstant) } } } diff --git a/src/sema/function_annotation.rs b/src/sema/function_annotation.rs index 2a669fb9b1..2d3c63db8a 100644 --- a/src/sema/function_annotation.rs +++ b/src/sema/function_annotation.rs @@ -1,15 +1,15 @@ // SPDX-License-Identifier: Apache-2.0 use super::{ - ast::{ConstructorAnnotation, Diagnostic, Expression, Function, Namespace, Type}, + ast::{Diagnostic, Expression, Function, Namespace, Type}, diagnostics::Diagnostics, eval::overflow_diagnostic, expression::literals::{hex_number_literal, unit_literal}, expression::{ExprContext, ResolveTo}, Symtable, }; -use crate::sema::ast::SolanaAccount; -use crate::sema::eval::eval_const_number; +use crate::sema::ast::{ConstructorAnnotations, SolanaAccount}; +use crate::sema::eval::{eval_const_number, EvaluationError}; use crate::sema::expression::literals::number_literal; use crate::sema::expression::resolve_expression::expression; use crate::sema::solana_accounts::BuiltinAccounts; @@ -185,11 +185,9 @@ pub(super) fn function_body_annotations( // @bump(param2) // constructor(bytes param1, uint8 param2) {} - let mut resolved_annotations = Vec::new(); - let mut bump = None; - let mut space = None; - let mut payer = None; + let mut has_annotation = false; + let mut annotations = ConstructorAnnotations::default(); let is_solana_constructor = ns.target == Target::Solana && ns.functions[function_no].ty == pt::FunctionTy::Constructor; @@ -202,19 +200,23 @@ pub(super) fn function_body_annotations( "seed" if is_solana_constructor => { let ty = Type::Slice(Box::new(Type::Bytes(1))); - let mut fake_loc = None; + let mut resolved = None; body_annotation( note.id.name.as_str(), &ty, - &mut fake_loc, + &mut resolved, note, &mut diagnostics, - &mut resolved_annotations, context, ns, symtable, + &mut has_annotation, ); + + if let Some(resolved) = resolved { + annotations.seeds.push(resolved); + } } "bump" if is_solana_constructor => { let ty = Type::Bytes(1); @@ -222,13 +224,13 @@ pub(super) fn function_body_annotations( body_annotation( note.id.name.as_str(), &ty, - &mut bump, + &mut annotations.bump, note, &mut diagnostics, - &mut resolved_annotations, context, ns, symtable, + &mut has_annotation, ); } "space" if is_solana_constructor => { @@ -237,13 +239,13 @@ pub(super) fn function_body_annotations( body_annotation( note.id.name.as_str(), &ty, - &mut space, + &mut annotations.space, note, &mut diagnostics, - &mut resolved_annotations, context, ns, symtable, + &mut has_annotation, ); } "payer" if is_solana_constructor => { @@ -277,7 +279,7 @@ pub(super) fn function_body_annotations( )); } Entry::Vacant(vacancy) => { - if let Some(prev) = &payer { + if let Some((prev, _)) = &annotations.payer { duplicate_annotation( &mut diagnostics, "payer", @@ -286,15 +288,13 @@ pub(super) fn function_body_annotations( ns.functions[function_no].ty.as_str(), ); } else { - payer = Some(loc); vacancy.insert(SolanaAccount { loc: note.loc, is_signer: true, is_writer: true, generated: false, }); - resolved_annotations - .push(ConstructorAnnotation::Payer(loc, id.name.clone())); + annotations.payer = Some((loc, id.name.clone())); } } } @@ -326,17 +326,21 @@ pub(super) fn function_body_annotations( { "seed" => { let ty = Type::Slice(Box::new(Type::Bytes(1))); - let mut fake_loc = None; + let mut resolved = None; parameter_annotation( function_no, unresolved, &ty, - &mut fake_loc, + &mut resolved, &mut diagnostics, - &mut resolved_annotations, ns, symtable, + &mut has_annotation, ); + + if let Some(resolved) = resolved { + annotations.seeds.push(resolved); + } } "bump" => { let ty = Type::Bytes(1); @@ -344,11 +348,11 @@ pub(super) fn function_body_annotations( function_no, unresolved, &ty, - &mut bump, + &mut annotations.bump, &mut diagnostics, - &mut resolved_annotations, ns, symtable, + &mut has_annotation, ); } "space" => { @@ -357,11 +361,11 @@ pub(super) fn function_body_annotations( function_no, unresolved, &ty, - &mut space, + &mut annotations.space, &mut diagnostics, - &mut resolved_annotations, ns, symtable, + &mut has_annotation, ); } @@ -392,7 +396,7 @@ pub(super) fn function_body_annotations( } } - if !resolved_annotations.is_empty() && diagnostics.is_empty() && payer.is_none() { + if has_annotation && diagnostics.is_empty() && annotations.payer.is_none() { diagnostics.push(Diagnostic::error( ns.functions[function_no].loc, "@payer annotation required for constructor".into(), @@ -401,43 +405,22 @@ pub(super) fn function_body_annotations( ns.diagnostics.extend(diagnostics); - ns.functions[function_no].annotations = resolved_annotations; + ns.functions[function_no].annotations = annotations; } /// Resolve the body annotations fn body_annotation( name: &str, ty: &Type, - previous: &mut Option, + resolved_annotation: &mut Option<(pt::Loc, Expression)>, annotation: &Annotation, diagnostics: &mut Diagnostics, - resolved_annotations: &mut Vec, context: &ExprContext, ns: &mut Namespace, symtable: &mut Symtable, + has_annotation: &mut bool, ) { let annotation_value = annotation.value.as_ref().unwrap(); - let mut dry_run = Diagnostics::default(); - if let Ok(expr) = expression( - annotation_value, - context, - ns, - symtable, - &mut dry_run, - ResolveTo::Type(ty), - ) { - // We only accept literals or constant expressions. - if !annotation_value.is_literal() && eval_const_number(&expr, ns, &mut dry_run).is_err() { - diagnostics.push(Diagnostic::error( - annotation.value.as_ref().unwrap().loc(), - format!( - "'@{}' annotation on top of a constructor only accepts literals", - name - ), - )); - return; - } - } if let Ok(expr) = expression( annotation_value, @@ -447,12 +430,33 @@ fn body_annotation( diagnostics, ResolveTo::Type(ty), ) { - if let Ok(expr) = expr.cast(&expr.loc(), ty, true, ns, diagnostics) { - if let Some(prev) = previous { + if let Ok(expr) = expr.cast(&annotation.loc, ty, true, ns, diagnostics) { + if let Some((prev, _)) = resolved_annotation { duplicate_annotation(diagnostics, name, expr.loc(), *prev, "constructor"); + } else if annotation_value.is_literal() { + *has_annotation = true; + *resolved_annotation = Some((annotation.loc, expr)); } else { - *previous = Some(annotation.loc); - resolved_annotations.push(ConstructorAnnotation::initialize_annotation(name, expr)); + let mut eval_diagnostics = Diagnostics::default(); + match eval_const_number(&expr, ns, &mut eval_diagnostics) { + Ok((_, _)) => { + *has_annotation = true; + *resolved_annotation = Some((annotation.loc, expr)); + } + Err(EvaluationError::MathError) => { + diagnostics.extend(eval_diagnostics); + } + + Err(EvaluationError::NotAConstant) => { + diagnostics.push(Diagnostic::error( + annotation.value.as_ref().unwrap().loc(), + format!( + "'@{}' annotation on top of a constructor only accepts literals", + name + ), + )); + } + } } } } @@ -463,15 +467,15 @@ fn parameter_annotation( function_no: usize, unresolved_annotation: &UnresolvedAnnotation, ty: &Type, - previous: &mut Option, + resolved_annotation: &mut Option<(pt::Loc, Expression)>, diagnostics: &mut Diagnostics, - resolved_annotations: &mut Vec, ns: &mut Namespace, symtable: &mut Symtable, + has_annotation: &mut bool, ) { let parameter = &ns.functions[function_no].params[unresolved_annotation.parameter_no]; let annotation = parameter.annotation.as_ref().unwrap(); - if let Some(prev) = previous { + if let Some((prev, _)) = resolved_annotation { duplicate_annotation( diagnostics, annotation.id.name.as_str(), @@ -496,22 +500,8 @@ fn parameter_annotation( .read = true; if let Ok(casted) = expr.cast(&annotation.loc, ty, true, ns, diagnostics) { - *previous = Some(annotation.loc); - resolved_annotations.push(ConstructorAnnotation::initialize_annotation( - annotation.id.name.as_str(), - casted, - )); - } -} - -impl ConstructorAnnotation { - fn initialize_annotation(name: &str, value: Expression) -> ConstructorAnnotation { - match name { - "seed" => ConstructorAnnotation::Seed(value), - "space" => ConstructorAnnotation::Space(value), - "bump" => ConstructorAnnotation::Bump(value), - _ => unreachable!("function should not be called with {}", name), - } + *has_annotation = true; + *resolved_annotation = Some((annotation.loc, casted)); } } diff --git a/tests/contract_testcases/solana/annotations/invalid_space.sol b/tests/contract_testcases/solana/annotations/invalid_space.sol new file mode 100644 index 0000000000..4a112d8e2c --- /dev/null +++ b/tests/contract_testcases/solana/annotations/invalid_space.sol @@ -0,0 +1,14 @@ +contract Test2 { + address public a; + address public b; + + @payer(acc) + @space(5<<64) + constructor(address c, address d) { + a = c; + b = d; + } +} + +// ---- Expect: diagnostics ---- +// error: codegen: value 92233720368547758208 does not fit into type uint64. \ No newline at end of file diff --git a/tests/contract_testcases/solana/annotations/not_enough_space.sol b/tests/contract_testcases/solana/annotations/not_enough_space.sol new file mode 100644 index 0000000000..795cba520b --- /dev/null +++ b/tests/contract_testcases/solana/annotations/not_enough_space.sol @@ -0,0 +1,16 @@ +contract Test { + address a; + address b; + + @payer(acc) + @space(5) + constructor(address c, address d) { + a = c; + b = d; + } +} + +// ---- Expect: diagnostics ---- +// warning: 2:5-14: storage variable 'a' has been assigned, but never read +// warning: 3:5-14: storage variable 'b' has been assigned, but never read +// error: 6:12-13: contract requires at least 80 bytes of space diff --git a/tests/contract_testcases/solana/annotations/not_enough_space_2.sol b/tests/contract_testcases/solana/annotations/not_enough_space_2.sol new file mode 100644 index 0000000000..8a5d24244a --- /dev/null +++ b/tests/contract_testcases/solana/annotations/not_enough_space_2.sol @@ -0,0 +1,14 @@ +contract Test3 { + address public a; + address public b; + + @payer(acc) + @space(5+8*2) + constructor(address c, address d) { + a = c; + b = d; + } +} + +// ---- Expect: diagnostics ---- +// error: 6:12-17: contract requires at least 80 bytes of space diff --git a/tests/solana_tests/create_contract.rs b/tests/solana_tests/create_contract.rs index cf592ce67b..fb9037d229 100644 --- a/tests/solana_tests/create_contract.rs +++ b/tests/solana_tests/create_contract.rs @@ -5,6 +5,7 @@ use crate::{ BorshToken, Pubkey, }; use base58::{FromBase58, ToBase58}; +use num_bigint::BigInt; #[test] fn simple_create_contract_no_seed() { @@ -750,3 +751,74 @@ contract Child { "Going to create childIn child constructorHello there" ); } + +#[test] +fn not_enough_space() { + let mut vm = build_solidity( + r#" + contract Test { + address a; + address b; + + @payer(acc) + constructor(address c, address d, @space uint64 mySpace) { + a = c; + b = d; + } +} + "#, + ); + + let data_account = account_new(); + vm.account_data + .insert(data_account, AccountState::default()); + let payer = account_new(); + vm.account_data.insert(payer, AccountState::default()); + let dummy_account_1 = account_new(); + let dummy_account_2 = account_new(); + + // This should work + vm.function("new") + .arguments(&[ + BorshToken::Address(dummy_account_1), + BorshToken::Address(dummy_account_2), + BorshToken::Uint { + width: 64, + value: BigInt::from(120), + }, + ]) + .accounts(vec![ + ("dataAccount", data_account), + ("acc", payer), + ("systemProgram", [0; 32]), + ]) + .call(); + + let other_account = account_new(); + vm.account_data + .insert(other_account, AccountState::default()); + + // This must fail + let res = vm + .function("new") + .arguments(&[ + BorshToken::Address(dummy_account_1), + BorshToken::Address(dummy_account_2), + BorshToken::Uint { + width: 64, + value: BigInt::from(8), + }, + ]) + .accounts(vec![ + ("dataAccount", other_account), + ("acc", payer), + ("systemProgram", [0; 32]), + ]) + .must_fail(); + + assert_eq!(res.unwrap(), 5u64 << 32); + assert_eq!( + vm.logs, + "value passed for space is insufficient. Contract requires at least 80 bytes" + ); +}