Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: added access modifier restrictions on clarity trait definitions #5383

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

feat: added access modifier restrictions on clarity trait definitions #5383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0912170
feat: added access modifier restrictions on clarity trait definitions
usagi32 Oct 24, 2024
8838481
Merge branch 'develop' into feat/clarity-trait-access-modifier
jcnelson Nov 8, 2024
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
4 changes: 2 additions & 2 deletions clarity/src/vm/analysis/analysis_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::vm::database::{
ClarityBackingStore, ClarityDeserializable, ClaritySerializable, RollbackWrapper,
};
use crate::vm::representations::ClarityName;
use crate::vm::types::signatures::FunctionSignature;
use crate::vm::types::signatures::{FunctionSignature, MethodSignature};
use crate::vm::types::{FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature};
use crate::vm::ClarityVersion;

Expand Down Expand Up @@ -207,7 +207,7 @@ impl<'a> AnalysisDatabase<'a> {
contract_identifier: &QualifiedContractIdentifier,
trait_name: &str,
epoch: &StacksEpochId,
) -> CheckResult<Option<BTreeMap<ClarityName, FunctionSignature>>> {
) -> CheckResult<Option<BTreeMap<ClarityName, MethodSignature>>> {
// TODO: this function loads the whole contract to obtain the function type.
// but it doesn't need to -- rather this information can just be
// stored as its own entry. the analysis cost tracking currently only
Expand Down
8 changes: 4 additions & 4 deletions clarity/src/vm/analysis/type_checker/v2_05/contexts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::vm::analysis::errors::{CheckError, CheckErrors, CheckResult};
use crate::vm::analysis::types::ContractAnalysis;
use crate::vm::contexts::MAX_CONTEXT_DEPTH;
use crate::vm::representations::{ClarityName, SymbolicExpression};
use crate::vm::types::signatures::FunctionSignature;
use crate::vm::types::signatures::{FunctionSignature, MethodSignature};
use crate::vm::types::{FunctionType, TraitIdentifier, TypeSignature};

pub struct ContractContext {
Expand All @@ -34,7 +34,7 @@ pub struct ContractContext {
persisted_variable_types: HashMap<ClarityName, TypeSignature>,
fungible_tokens: HashSet<ClarityName>,
non_fungible_tokens: HashMap<ClarityName, TypeSignature>,
traits: HashMap<ClarityName, BTreeMap<ClarityName, FunctionSignature>>,
traits: HashMap<ClarityName, BTreeMap<ClarityName, MethodSignature>>,
pub implemented_traits: HashSet<TraitIdentifier>,
}

Expand Down Expand Up @@ -170,7 +170,7 @@ impl ContractContext {
pub fn add_trait(
&mut self,
trait_name: ClarityName,
trait_signature: BTreeMap<ClarityName, FunctionSignature>,
trait_signature: BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
self.traits.insert(trait_name, trait_signature);
Ok(())
Expand All @@ -181,7 +181,7 @@ impl ContractContext {
Ok(())
}

pub fn get_trait(&self, trait_name: &str) -> Option<&BTreeMap<ClarityName, FunctionSignature>> {
pub fn get_trait(&self, trait_name: &str) -> Option<&BTreeMap<ClarityName, MethodSignature>> {
self.traits.get(trait_name)
}

Expand Down
6 changes: 3 additions & 3 deletions clarity/src/vm/analysis/type_checker/v2_05/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use crate::vm::representations::SymbolicExpressionType::{
Atom, AtomValue, Field, List, LiteralValue, TraitReference,
};
use crate::vm::representations::{depth_traverse, ClarityName, SymbolicExpression};
use crate::vm::types::signatures::{FunctionSignature, BUFF_20};
use crate::vm::types::signatures::{FunctionSignature, MethodSignature, BUFF_20};
use crate::vm::types::{
parse_name_type_pairs, FixedFunction, FunctionArg, FunctionType, PrincipalData,
QualifiedContractIdentifier, TupleTypeSignature, TypeSignature, Value,
Expand Down Expand Up @@ -299,7 +299,7 @@ impl FunctionType {
}
}

fn trait_type_size(trait_sig: &BTreeMap<ClarityName, FunctionSignature>) -> CheckResult<u64> {
fn trait_type_size(trait_sig: &BTreeMap<ClarityName, MethodSignature>) -> CheckResult<u64> {
let mut total_size = 0;
for (_func_name, value) in trait_sig.iter() {
total_size = total_size.cost_overflow_add(value.total_type_size()?)?;
Expand Down Expand Up @@ -808,7 +808,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
trait_name: &ClarityName,
function_types: &[SymbolicExpression],
_context: &mut TypingContext,
) -> CheckResult<(ClarityName, BTreeMap<ClarityName, FunctionSignature>)> {
) -> CheckResult<(ClarityName, BTreeMap<ClarityName, MethodSignature>)> {
let trait_signature = TypeSignature::parse_trait_type_repr(
function_types,
&mut (),
Expand Down
9 changes: 6 additions & 3 deletions clarity/src/vm/analysis/type_checker/v2_05/natives/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,21 +413,24 @@ fn check_contract_call(
let trait_signature = checker.contract_context.get_trait(&trait_id.name).ok_or(
CheckErrors::TraitReferenceUnknown(trait_id.name.to_string()),
)?;
let func_signature =
trait_signature
let func_signature = {
let method_sign = trait_signature
.get(func_name)
.ok_or(CheckErrors::TraitMethodUnknown(
trait_id.name.to_string(),
func_name.to_string(),
))?;

FunctionSignature { args: method_sign.args.clone(), returns: method_sign.returns.clone() }
};

runtime_cost(
ClarityCostFunction::AnalysisLookupFunctionTypes,
&mut checker.cost_track,
func_signature.total_type_size()?,
)?;

func_signature.clone()
func_signature
}
_ => return Err(CheckError::new(CheckErrors::ContractCallExpectName)),
};
Expand Down
18 changes: 9 additions & 9 deletions clarity/src/vm/analysis/type_checker/v2_1/contexts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,19 @@ use crate::vm::analysis::type_checker::is_reserved_word;
use crate::vm::analysis::types::ContractAnalysis;
use crate::vm::contexts::MAX_CONTEXT_DEPTH;
use crate::vm::representations::{ClarityName, SymbolicExpression};
use crate::vm::types::signatures::{CallableSubtype, FunctionSignature};
use crate::vm::types::signatures::{CallableSubtype, FunctionSignature, MethodSignature};
use crate::vm::types::{FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature};
use crate::vm::ClarityVersion;

enum TraitContext {
/// Traits stored in this context use the trait type-checking behavior defined in Clarity1
Clarity1(HashMap<ClarityName, BTreeMap<ClarityName, FunctionSignature>>),
Clarity1(HashMap<ClarityName, BTreeMap<ClarityName, MethodSignature>>),
/// Traits stored in this context use the new trait type-checking behavior defined in Clarity2
Clarity2 {
/// Aliases for locally defined traits and traits imported with `use-trait`
defined: HashSet<ClarityName>,
/// All traits which are defined or used in a contract
all: HashMap<TraitIdentifier, BTreeMap<ClarityName, FunctionSignature>>,
all: HashMap<TraitIdentifier, BTreeMap<ClarityName, MethodSignature>>,
},
}

Expand All @@ -61,7 +61,7 @@ impl TraitContext {
&mut self,
contract_identifier: QualifiedContractIdentifier,
trait_name: ClarityName,
trait_signature: BTreeMap<ClarityName, FunctionSignature>,
trait_signature: BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
match self {
Self::Clarity1(map) => {
Expand All @@ -85,7 +85,7 @@ impl TraitContext {
&mut self,
alias: ClarityName,
trait_id: TraitIdentifier,
trait_signature: BTreeMap<ClarityName, FunctionSignature>,
trait_signature: BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
match self {
Self::Clarity1(map) => {
Expand All @@ -102,7 +102,7 @@ impl TraitContext {
pub fn get_trait(
&self,
trait_id: &TraitIdentifier,
) -> Option<&BTreeMap<ClarityName, FunctionSignature>> {
) -> Option<&BTreeMap<ClarityName, MethodSignature>> {
match self {
Self::Clarity1(map) => map.get(&trait_id.name),
Self::Clarity2 { defined: _, all } => all.get(trait_id),
Expand Down Expand Up @@ -284,7 +284,7 @@ impl ContractContext {
pub fn add_defined_trait(
&mut self,
trait_name: ClarityName,
trait_signature: BTreeMap<ClarityName, FunctionSignature>,
trait_signature: BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
if self.clarity_version >= ClarityVersion::Clarity3 {
self.check_name_used(&trait_name)?;
Expand All @@ -301,7 +301,7 @@ impl ContractContext {
&mut self,
alias: ClarityName,
trait_id: TraitIdentifier,
trait_signature: BTreeMap<ClarityName, FunctionSignature>,
trait_signature: BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
if self.clarity_version >= ClarityVersion::Clarity3 {
self.check_name_used(&alias)?;
Expand All @@ -318,7 +318,7 @@ impl ContractContext {
pub fn get_trait(
&self,
trait_id: &TraitIdentifier,
) -> Option<&BTreeMap<ClarityName, FunctionSignature>> {
) -> Option<&BTreeMap<ClarityName, MethodSignature>> {
self.traits.get_trait(trait_id)
}

Expand Down
16 changes: 8 additions & 8 deletions clarity/src/vm/analysis/type_checker/v2_1/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::vm::representations::SymbolicExpressionType::{
};
use crate::vm::representations::{depth_traverse, ClarityName, SymbolicExpression};
use crate::vm::types::signatures::{
CallableSubtype, FunctionArgSignature, FunctionReturnsSignature, FunctionSignature, BUFF_20,
CallableSubtype, FunctionArgSignature, FunctionReturnsSignature, FunctionSignature, MethodSignature, BUFF_20
};
use crate::vm::types::{
parse_name_type_pairs, CallableData, FixedFunction, FunctionArg, FunctionType, ListData,
Expand Down Expand Up @@ -647,8 +647,8 @@ fn check_function_arg_signature<T: CostTracker>(
fn clarity2_check_functions_compatible<T: CostTracker>(
db: &mut AnalysisDatabase,
contract_context: Option<&ContractContext>,
expected_sig: &FunctionSignature,
actual_sig: &FunctionSignature,
expected_sig: &MethodSignature,
actual_sig: &MethodSignature,
tracker: &mut T,
) -> bool {
if expected_sig.args.len() != actual_sig.args.len() {
Expand Down Expand Up @@ -692,9 +692,9 @@ pub fn clarity2_trait_check_trait_compliance<T: CostTracker>(
db: &mut AnalysisDatabase,
contract_context: Option<&ContractContext>,
actual_trait_identifier: &TraitIdentifier,
actual_trait: &BTreeMap<ClarityName, FunctionSignature>,
actual_trait: &BTreeMap<ClarityName, MethodSignature>,
expected_trait_identifier: &TraitIdentifier,
expected_trait: &BTreeMap<ClarityName, FunctionSignature>,
expected_trait: &BTreeMap<ClarityName, MethodSignature>,
tracker: &mut T,
) -> CheckResult<()> {
// Shortcut for the simple case when the two traits are the same.
Expand Down Expand Up @@ -910,7 +910,7 @@ fn clarity2_lookup_trait<T: CostTracker>(
contract_context: Option<&ContractContext>,
trait_id: &TraitIdentifier,
tracker: &mut T,
) -> CheckResult<BTreeMap<ClarityName, FunctionSignature>> {
) -> CheckResult<BTreeMap<ClarityName, MethodSignature>> {
if let Some(contract_context) = contract_context {
// If the trait is from this contract, then it must be in the context or it doesn't exist.
if contract_context.is_contract(&trait_id.contract_identifier) {
Expand Down Expand Up @@ -956,7 +956,7 @@ fn clarity2_lookup_trait<T: CostTracker>(
}
}

fn trait_type_size(trait_sig: &BTreeMap<ClarityName, FunctionSignature>) -> CheckResult<u64> {
fn trait_type_size(trait_sig: &BTreeMap<ClarityName, MethodSignature>) -> CheckResult<u64> {
let mut total_size = 0;
for (_func_name, value) in trait_sig.iter() {
total_size = total_size.cost_overflow_add(value.total_type_size()?)?;
Expand Down Expand Up @@ -1610,7 +1610,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
trait_name: &ClarityName,
function_types: &[SymbolicExpression],
_context: &mut TypingContext,
) -> CheckResult<(ClarityName, BTreeMap<ClarityName, FunctionSignature>)> {
) -> CheckResult<(ClarityName, BTreeMap<ClarityName, MethodSignature>)> {
let trait_signature = TypeSignature::parse_trait_type_repr(
function_types,
&mut (),
Expand Down
26 changes: 16 additions & 10 deletions clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,21 +499,23 @@ fn check_contract_call(
let trait_signature = checker.contract_context.get_trait(trait_id).ok_or(
CheckErrors::TraitReferenceUnknown(trait_id.name.to_string()),
)?;
let func_signature =
trait_signature
let func_signature = {
let method_sign = trait_signature
.get(func_name)
.ok_or(CheckErrors::TraitMethodUnknown(
trait_id.name.to_string(),
func_name.to_string(),
))?;

FunctionSignature { args: method_sign.args.clone(), returns: method_sign.returns.clone() }
};
runtime_cost(
ClarityCostFunction::AnalysisLookupFunctionTypes,
&mut checker.cost_track,
func_signature.total_type_size()?,
)?;

func_signature.clone()
func_signature
} else {
// Clarity2+
match checker.contract_context.get_variable_type(trait_instance) {
Expand Down Expand Up @@ -577,20 +579,24 @@ fn check_contract_call(
let trait_signature = checker.contract_context.get_trait(trait_id).ok_or(
CheckErrors::TraitReferenceUnknown(trait_id.name.to_string()),
)?;
let func_signature = trait_signature.get(func_name).ok_or(
CheckErrors::TraitMethodUnknown(
trait_id.name.to_string(),
func_name.to_string(),
),
)?;
let func_signature = {
let method_sign = trait_signature
.get(func_name)
.ok_or(CheckErrors::TraitMethodUnknown(
trait_id.name.to_string(),
func_name.to_string(),
))?;

FunctionSignature { args: method_sign.args.clone(), returns: method_sign.returns.clone() }
};

runtime_cost(
ClarityCostFunction::AnalysisLookupFunctionTypes,
&mut checker.cost_track,
func_signature.total_type_size()?,
)?;

func_signature.clone()
func_signature
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions clarity/src/vm/analysis/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::vm::analysis::contract_interface_builder::ContractInterface;
use crate::vm::analysis::errors::{CheckErrors, CheckResult};
use crate::vm::analysis::type_checker::contexts::TypeMap;
use crate::vm::costs::{CostTracker, ExecutionCost, LimitedCostTracker};
use crate::vm::types::signatures::FunctionSignature;
use crate::vm::types::signatures::{FunctionSignature, MethodSignature, MethodType};
use crate::vm::types::{FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature};
use crate::vm::{ClarityName, ClarityVersion, SymbolicExpression};

Expand Down Expand Up @@ -52,7 +52,7 @@ pub struct ContractAnalysis {
pub persisted_variable_types: BTreeMap<ClarityName, TypeSignature>,
pub fungible_tokens: BTreeSet<ClarityName>,
pub non_fungible_tokens: BTreeMap<ClarityName, TypeSignature>,
pub defined_traits: BTreeMap<ClarityName, BTreeMap<ClarityName, FunctionSignature>>,
pub defined_traits: BTreeMap<ClarityName, BTreeMap<ClarityName, MethodSignature>>,
pub implemented_traits: BTreeSet<TraitIdentifier>,
pub contract_interface: Option<ContractInterface>,
pub is_cost_contract_eligible: bool,
Expand Down Expand Up @@ -153,7 +153,7 @@ impl ContractAnalysis {
pub fn add_defined_trait(
&mut self,
name: ClarityName,
function_types: BTreeMap<ClarityName, FunctionSignature>,
function_types: BTreeMap<ClarityName, MethodSignature>,
) {
self.defined_traits.insert(name, function_types);
}
Expand Down Expand Up @@ -189,7 +189,7 @@ impl ContractAnalysis {
pub fn get_defined_trait(
&self,
name: &str,
) -> Option<&BTreeMap<ClarityName, FunctionSignature>> {
) -> Option<&BTreeMap<ClarityName, MethodSignature>> {
self.defined_traits.get(name)
}

Expand Down Expand Up @@ -228,17 +228,20 @@ impl ContractAnalysis {
&self,
epoch: &StacksEpochId,
trait_identifier: &TraitIdentifier,
trait_definition: &BTreeMap<ClarityName, FunctionSignature>,
trait_definition: &BTreeMap<ClarityName, MethodSignature>,
) -> CheckResult<()> {
let trait_name = trait_identifier.name.to_string();

for (func_name, expected_sig) in trait_definition.iter() {
match (
self.get_public_function_type(func_name),
self.get_read_only_function_type(func_name),
&expected_sig.define_type,
) {
(Some(FunctionType::Fixed(func)), None)
| (None, Some(FunctionType::Fixed(func))) => {
(None, Some(FunctionType::Fixed(func)), MethodType::ReadOnly)
| (Some(FunctionType::Fixed(func)), None, MethodType::Public)
| (None, Some(FunctionType::Fixed(func)), MethodType::NotDefined)
| (Some(FunctionType::Fixed(func)), None, MethodType::NotDefined) => {
let args_sig = func.args.iter().map(|a| a.signature.clone()).collect();
if !expected_sig.check_args_trait_compliance(epoch, args_sig)? {
return Err(CheckErrors::BadTraitImplementation(
Expand All @@ -256,7 +259,7 @@ impl ContractAnalysis {
.into());
}
}
(_, _) => {
(_, _, _) => {
return Err(CheckErrors::BadTraitImplementation(
trait_name,
func_name.to_string(),
Expand Down Expand Up @@ -296,12 +299,13 @@ mod test {
let mut trait_functions = BTreeMap::new();
trait_functions.insert(
"alpha".into(),
FunctionSignature {
MethodSignature {
args: vec![TypeSignature::TraitReferenceType(trait_id.clone())],
returns: TypeSignature::ResponseType(Box::new((
TypeSignature::UIntType,
TypeSignature::UIntType,
))),
define_type: MethodType::NotDefined,
},
);
contract_analysis.add_defined_trait("foo".into(), trait_functions);
Expand Down
Loading