You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The logic determines the current stage dynamically, based on the value of multiple state variables:
self.nr_winning_tickets()
self.total_confirmed_tickets()
self.claim_start_epoch()
self.winner_selection_start_epoch()
self.current_generation()
self.confirmation_period_start_epoch()
self.confirmation_period_in_epochs()
Some of these variables can be changed by the owner at will (with some limitations).
The issue is that the logic that determines the current stage makes use of dynamic variables that can be and are changed during the lifecycle of this contract:
let total_winning_tickets = self.nr_winning_tickets().get();
let total_confirmed_tickets = self.get_total_confirmed_tickets();
if total_confirmed_tickets >= total_winning_tickets {
let claim_start_epoch = self.claim_start_epoch().get();
if current_epoch >= claim_start_epoch {
returnLaunchStage::Claim;
}else{
returnLaunchStage::WaitBeforeClaim;
}
}
let winner_selection_start_epoch = self.winner_selection_start_epoch().get();
if current_epoch < winner_selection_start_epoch {
returnLaunchStage::AddTickets;
}
Compared to an explicit logic whereby the owner can manage the stage transitions manually (and the contract code validates that each transition is valid), the current version of the code is more prone to errors. An invalid state transition (see #5) was already found but was difficult to spot because of the way the get_launch_stage function uses dynamic variables that can be changed in other parts of the contract.
The documentation features a state diagram that outlines the transition flow of stages:
Below is an example of how this flow chart does not necessarily represent what the code can do:
Current stage is ConfirmTickets (ie. confirmation_period_start_epoch < current_epoch < confiration_period_end_epoch)
Owner updates the following state variables so that current_epoch < winner_selection_start_epoch: winner_selection_start_epoch, confirmation_period_start_epoch
Current stage is AddTickets - the code allowed the owner to perform a transition from ConfirmTickets to AddTickets which is not reflected in the flowchart
Further on, this invalid transition can pose a risk since adding more tickets in its current state might expose other issues
Recommendation
We prefer a more explicit way of managing the state. If resources allow we recommend rewriting the get_launch_stage function to allow the owner to set the current stage and only do sanity checks to ensure that the state transition is valid.
The text was updated successfully, but these errors were encountered:
Description
The Elrond Launchpad contract LaunchStage selection relies on the
get_launch_stage
function which returns the current stage:review-elrond-launchpad-2021-10/code/launchpad/src/launchpad.rs
Lines 379 to 380 in a32fe0b
This function is used to restrict actions only to specific stages:
review-elrond-launchpad-2021-10/code/launchpad/src/launchpad.rs
Lines 292 to 294 in a32fe0b
The logic determines the current stage dynamically, based on the value of multiple state variables:
self.nr_winning_tickets()
self.total_confirmed_tickets()
self.claim_start_epoch()
self.winner_selection_start_epoch()
self.current_generation()
self.confirmation_period_start_epoch()
self.confirmation_period_in_epochs()
Some of these variables can be changed by the owner at will (with some limitations).
The issue is that the logic that determines the current stage makes use of dynamic variables that can be and are changed during the lifecycle of this contract:
review-elrond-launchpad-2021-10/code/launchpad/src/launchpad.rs
Lines 383 to 397 in a32fe0b
Compared to an explicit logic whereby the owner can manage the stage transitions manually (and the contract code validates that each transition is valid), the current version of the code is more prone to errors. An invalid state transition (see #5) was already found but was difficult to spot because of the way the
get_launch_stage
function uses dynamic variables that can be changed in other parts of the contract.The documentation features a state diagram that outlines the transition flow of stages:
Below is an example of how this flow chart does not necessarily represent what the code can do:
ConfirmTickets
(ie.confirmation_period_start_epoch < current_epoch < confiration_period_end_epoch
)current_epoch < winner_selection_start_epoch
:winner_selection_start_epoch
,confirmation_period_start_epoch
AddTickets
- the code allowed the owner to perform a transition from ConfirmTickets to AddTickets which is not reflected in the flowchartRecommendation
We prefer a more explicit way of managing the state. If resources allow we recommend rewriting the
get_launch_stage
function to allow the owner to set the current stage and only do sanity checks to ensure that the state transition is valid.The text was updated successfully, but these errors were encountered: