Category: Coding Mistakes
Unclear behavior of the function set_modules
Low Severity
Low Impact
Medium Likelihood
Description
The following is the code of the Aggregation ISM:
fn set_modules(ref self: ContractState, _modules: Span<ContractAddress>) {
self.ownable.assert_only_owner();
assert(!self.are_modules_stored(_modules), Errors::MODULES_ALREADY_STORED);
let mut last_module = self.find_last_module();
let mut cur_idx = 0;
loop {
if (cur_idx == _modules.len()) {
break ();
}
let module = *_modules.at(cur_idx);
assert(
module != contract_address_const::<0>(), Errors::MODULE_ADDRESS_CANNOT_BE_NULL
);
self.modules.write(last_module, module);
cur_idx += 1;
last_module = module;
}
}This function appends the given modules to the existing list of modules; not replacing the existing list of modules.
Impact
This inconsistency between the function name and its behavior could lead to potential misuse.
Recommendations
Consider changing the behavior of this function or renaming this function to match the function name to its behavior.
Remediation
This issue has been acknowledged by Pragma, and a fix was implemented in commit 6ef5aa2e↗.