🔐
Zellic Audit
February 22, 2023
Prepared for: Multisig Labs
Prepared by: Katerina Belotskaia and Vlad Toie
Zellic Inc.
Zellic was founded in 2020 by a team of blockchain specialists with more than a decade of combined industry experience. We are leading experts in smart contracts and Web3 development, cryptography, web security, and reverse engineering. Before Zellic, we founded perfect blue, the top competitive hacking team in the world. Since then, our team has won countless cybersecurity contests and blockchain security events.
Zellic aims to treat clients on a case-by-case basis and to consider their individual, unique concerns and business needs. Our goal is to see the long-term success of our partners rather than simply provide a list of present security issues. Similarly, we strive to adapt to our partners’ timelines and to be as available as possible. To keep up with our latest endeavors and research, check out our website zellic.io or follow @zellic_io on Twitter. If you are interested in partnering with Zellic, please email us at [email protected] or contact us on Telegram at https://t.me/zellic_io.
Zellic conducted an audit for Multisig Labs from November 14th to 29th, 2022.
Our general overview of the code is that it was well-organized and structured. The code coverage is high, and tests are included for the majority of the functions. Some areas of the code have limited negative testing, which could be improved. The documentation was adequate, although it could be improved. The code was easy to comprehend, and in most cases, intuitive.
Zellic thoroughly reviewed the GoGoPool codebase to find protocol-breaking bugs as defined by the documentation and to find any technical issues outlined in the Methodology section (2.2) of this document.
Specifically, taking into account GoGoPool’s threat model, we focused heavily on issues that would break core invariants such as the management of the minipools, staking, withdrawing and minting shares, and the states of the Storage contract.
During our assessment on the scoped GoGoPool contracts, we discovered seven findings. Of the seven findings, four were of high severity, one was of medium severity, one was of low severity and the remaining finding was informational.
Additionally, Zellic recorded its notes and observations from the audit for Multisig Labs’s benefit in the Discussion section ( 4 ) at the end of the document.
Impact Level | Count |
|---|---|
Critical | 0 |
High | 4 |
Medium | 1 |
Low | 1 |
Informational | 1 |
GoGoPool allows Avalanche users to stake a minimum of 0.01 AVAX and operate a validator node with a minimum of 1000 AVAX, while providing instant liquidity and earning rewards for validating subnets. As an open protocol, any individual, business, or subnet can plug into the protocol without being charged platform fees.
During a security assessment, Zellic works through standard phases of security auditing including both automated testing and manual review. These processes can vary significantly per engagement, but the majority of the time is spent on a thorough manual review of the entire scope.
Alongside a variety of open-source tools and analyzers used on an as-needed basis, Zellic focuses primarily on the following classes of security and reliability issues:
Basic coding mistakes. Many critical vulnerabilities in the past have been caused by simple, surface-level mistakes that could have easily been caught ahead of time by code review. We analyze the scoped smart contract code using automated tools to quickly sieve out and catch these shallow bugs. Depending on the engagement, we may also employ sophisticated analyzers such as model checkers, theorem provers, fuzzers, and so forth as necessary. We also perform a cursory review of the code to familiarize ourselves with the contracts.
Business logic errors. Business logic is the heart of any smart contract application. We manually review the contract logic to ensure that the code implements the expected functionality as specified in the platform’s design documents. We also thoroughly examine the specifications and designs themselves for inconsistencies, flaws, and vulnerabilities. This involves use cases that open the opportunity for abuse, such as flawed tokenomics or share pricing, arbitrage opportunities, and so forth.
Complex integration risks. Several high-profile exploits have not been the result of any bug within the contract itself; rather, they are an unintended consequence of the contract’s interaction with the broader DeFi ecosystem. We perform a meticulous review of all of the contract’s possible external interactions and summarize the associated risks: for example, flash loan attacks, oracle price manipulation, MEV/sandwich attacks, and so forth.
Codematurity. We review for possible improvements in the codebase in general. We look for violations of industry best practices and guidelines and code quality standards. We also provide suggestions for possible optimizations, such as gas optimization, upgradeability weaknesses, centralization risks, and so forth.
For each finding, Zellic assigns it an impact rating based on its severity and likelihood. There is no hard-and-fast formula for calculating a finding’s impact; we assign it on a case-by-case basis based on our professional judgment and experience. As one would expect, both the severity and likelihood of an issue affect its impact; for instance, a highly severe issue’s impact may be attenuated by a very low likelihood. We assign the following impact ratings (ordered by importance): Critical, High, Medium, Low, and Informational.
Similarly, Zellic organizes its reports such that the most important findings come first in the document rather than being ordered on impact alone. Thus, we may sometimes emphasize an “Informational” finding higher than a “Low” finding. The key distinction is that although certain findings may have the same impact rating, their importance may differ. This varies based on numerous soft factors, such as our clients’ threat models, their business needs, their project timelines, and so forth. We aim to provide useful and actionable advice to our partners that consider their long-term goals rather than simply provide a list of security issues at present.
The engagement involved a review of the following targets:
GoGoPool Contracts
Repository: https://github.com/multisig-labs/gogopool-contracts
Versions: 7768287e94bff0f2e12f03427309777e82a6e2fc
Contracts:
- BaseAbstract.sol
- RewardsPool.sol
- BaseUpgradeable.sol
- MultisigManager.sol
- Oracle.sol
- MinipoolManager.sol
- Vault.sol
- Storage.sol
- Base.sol
- ProtocolDAO.sol
- Ocyticus.sol
- tokens/TokenggAVAX.sol
- tokens/TokenGGP.sol
- tokens/upgradeable/ERC20Upgradeable.sol
- tokens/upgradeable/ERC4626Upgradeable.sol
- ClaimProtocolDAO.sol
- ClaimNodeOp.sol
- Staking.sol
Type: Solidity
Platform: EVM-compatible
Zellic was contracted to perform a security assessment with two consultants for a total of four person-weeks. The assessment was conducted over the course of two calendar weeks.
Contact Information
The following project managers were associated with the engagement:
Jasraj Bedi , Co-founder
Chad McDonald , Engagement Manager
The following consultants were engaged to conduct the assessment:
Katerina Belotskaia , Engineer
Vlad Toie , Engineer
The key dates of the engagement are detailed below.
November 14, 2022 Kick-off call
November 14, 2022 Start of primary review period
November 28, 2022 End of primary review period
- Target : Vault.sol
- Category : Business Logic
- Likelihood : Medium
- Severity : High
- Impact : High
The
transferAVAX function is used to perform transfers of avax between two registered contracts.function transferAVAX (
string memory fromContractName,
string memory toContractName,
uint256 amount
) external onlyRegisteredNetworkContract{
// Valid Amount?
if (amount == 0 ) {
revertInvalidAmount();
}
// Emit transfer event
emit AVAXTransfer(fromContractName, toContractName, amount);
// Make sure the contracts are valid, will revert if not
getContractAddress(fromContractName);
getContractAddress(toContractName);
// Verify there are enough funds
if (avaxBalances[fromContractName] < amount) {
revert InsufficientContractBalance();
}
// Update balances
avaxBalances[fromContractName] = avaxBalances[fromContractName] - amount;
avaxBalances[toContractName] = avaxBalances[toContractName] + amount;
}
The current checks ensure that the
msg.sender is a registeredNetworkContract; however, the function lacks a check on whether the msg.sender actually calls the function or not.Due to the fact that
fromContractName can be an arbitrary address, a presumably malicious registeredNetworkCContract can drain the avax balances of all the other registered contracts.We recommend removing the
fromContractName parameter altogether and ensuring that the funds can only be transferred by the caller of the function, msg.sender.// @audit-info doesn't exist in rocketvault
function transferAVAX (
string memory fromContractName,
string memory toContractName,
uint256 amount
) external onlyRegisteredNetworkContract {
// Valid Amount?
if(amount=) 0 ) {
revert InvalidAmount();
}
// Emit transfer event
emit AVAXTransfer(msg.sender, toContractName, amount);
// Make sure the contracts are valid, will revert if not
getContractAddress(msg.sender);
getContractAddress(toContractName);
// Verify there are enough funds
if (avaxBalances[msg.sender] < amount) {
revert InsufficientContractBalance();
}
// Update balances
avaxBalances[msg.sender] = avaxBalances[msg.sender] - amount;
avaxBalances[toContractName] = avaxBalances[toContractName] + amount;
}
The issue has been fixed by Multisig Labs in commit 84211f.
- Target : Ocyticus, Staking
- Category : Business Logic
- Likelihood : Medium
- Severity : High
- Impact : High
The
pauseEverything and resumeEverything functions are used to restrict access to important functions.function pauseEverything() external onlyDefender {
ProtocolDAO dao = ProtocolDAO(getContractAddress(“ProtocolDAO”));
dao.pauseContract(“TokenggAVAX”);
dao.pauseContract(“MinipoolManager”);
disableAllMultisigs();
}
/// @notice Reestablish all contract's abilities
/// @dev Multisigs will need to be enabled seperately, we dont know which ones to enable
function resumeEverything() external onlyDefender {
ProtocolDAO dao = ProtocolDAO(getContractAddress(“ProtocolDAO”));
dao.resumeContract(“TokenggAVAX”);
dao.resumeContract(“MinipoolManager”);
}
Apart from the
TokenGGAvax and MinipoolManager, the Staking contract also makes use of the whenNotPaused modifier for its important functions. The paused state, will, however, not trigger at the same time with the pauseEverything call, since the Staking contract is omitted here, both for pausing and resuming.Should an emergency arise,
pauseEverything will be called. In this case, Staking will be omitted, which could put user funds in danger.We recommend ensuring that the
Staking contract is also paused in the pauseEverything function as well as un-paused in the resumeEverything function.function pauseEverything() external onlyDefender {
ProtocolDAO dao = ProtocolDAO(getContractAddress(“ProtocolDAO”));
dao.pauseContract(“TokenggAVAX”);
dao.pauseContract(“MinipoolManager”);
dao.pauseContract(“Staking”);
disableAllMultisigs();
}
/// @notice Reestablish all contract's abilities
/// @dev Multisigs will need to be enabled separately, we don't know which ones to enable
function resumeEverything() external onlyDefender {
ProtocolDAO dao = ProtocolDAO(getContractAddress(“ProtocolDAO”));
dao.resumeContract(“TokenggAVAX”);
dao.resumeContract(“MinipoolManager”);
dao.resumeContract(“Staking”);
}
The issue has been fixed by Multisig Labs in commit dbc499.
- Target : ClaimNodeOp.sol
- Category : Business Logic
- Likelihood : Medium
- Severity : High
- Impact : High
A staker is eligible for the upcoming rewards cycle if they have staked their tokens for a long enough period of time. The reward amount is distributed in proportion to the amount of funds staked by the user from the total amount of funds staked by all users who claim the reward. But since the
rewardsStartTime is the time of creation of only the first pool, and during the reward calculations all staked funds are taken into account, even if they have not yet been blocked and can be withdrawn, the attack described below is possible.The attack scenario:
- 1.An attacker stakes ggp tokens and creates a minipool with a minimum
avaxAssignmentRequestvalue. - 2.The multisig initiates the staking process by calling the
claimAndInitiateStakingfunction. - 3.Wait for the time of distribution of rewards.
- 4.Before the reward distribution process begins, the attacker creates a new minipool with the maximum
avaxAssignmentRequestvalue. - 5.Initiate the reward distribution process.
- 6.Immediately after that, the attacker cancels the minipool with
cancelMinipoolfunction before theclaimAndInitiateStakingfunction call and returns most part of their staked funds.
The attacker can increase their reward portion without actually staking their own funds.
Take into account only the funds actually staked, or check that all minipools have been launched.
The issue has been fixed by Multisig Labs in commits c90b2f and f49931.
- Target : Project-wide
- Category : Business Logic
- Likelihood : Low
- Severity : High
- Impact : High
The network-registered contracts have absolute control over the storage that all the contracts are associated with through the
Storage contract. This is inherent due to the overall design of the protocol,which makes use of a single Storage contract eliminating the need of local storage. For that reason any registeredContract can update any storage slot even if it “belongs” to another contract.modifier onlyRegisteredNetworkContract() {
if (booleanStorage[keccak256(abi.encodePacked(“contract.exists”,
msg.sender))] == false && msg.sender != guardian) {
revertInvalidOrOutdatedContract();
}
_;
}
// ...
function setAddress (bytes32 key, address value) external onlyRegisteredNetworkContract {
address Storage[key] = value;
}
function setBool (bytes32 key, bool value) external onlyRegisteredNetworkContract {
boolean Storage[key] = value;
}
function setBytes(bytes32 key, bytes calldata value) external onlyRegisteredNetworkContract {
bytes Storage[key] = value;
}
As an example, the setter functions inside the
Staking contract have different restrictions for caller (e.g., the setLastRewardsCycleCompleted function can be called only by ClaimNodeOp contract), but actually the setUint function from it may be called by any RegisteredNetworkContract.We believe that in a highly unlikely case,a malicious
networkRegistered contract could potentially alter the entire protocol Storage to their will. Additionally, if it were possible to setBool of an arbitrary address, then this scenario would be further exploitable by a malicious developer contract.We recommend paying extra attention to the registration of
networkContracts, as well as closely monitoring where and when the setBool function is used, since the network registration is based on a boolean value attributed to the contract address.The issue has ben acknowledged by the Multisig Labs. Their official reply is reproduced below:
While it is true that any registered contract can write to Storage, we view all of the separate contracts comprising the Protocol as a single system. A single entity (either the Guardian Multisig or in future the ProtocolDAO) will be in control of all of the contracts. In this model, if an attacker can register a single malicious contract, then they are also in full control of the Protocol itself. Because all of the contracts are treated as a single entity, there is no additional security benefit to be gained by providing access controls between the various contract’s storage slots. As a mitigation, the Protocol will operate several distributed Watchers that will continually scan the central Storage contract, and alert on any changes.
- Target : Oracle
- Category : Business Logic
- Likelihood : Medium
- Severity : Medium
- Impact : Medium
Some functions at protocol-level make use of the
getGGPPriceInAvax. This getter retrieves the price, which is set by the Rialto multisig./// @notice Get the price of GGP denominated in AVAX
/// @return price of ggp in AVAX
/// @return timestamp representing when it was updated
function getGGPPriceInAVAX() external view returns (uint256 price, uint256 timestamp) {
price = getUint(keccak256(“Oracle.GGPPriceInAVAX”));
if(price == 0) {
revert InvalidGGPPrice();
}
timestamp = getUint(keccak256(“Oracle.GGPTimestamp”));
}
Due to the nature of on-chain price feeds,
Oracles need to have an as-often-as-possible policy in regards to how often the price gets updated. For that reason, the reliance on the Rialto may be problematic should it fail to update the price often enough.Should the price be erroneous, possible front-runs may happen at the protocol level, potentially leading to a loss of funds on the user-end side.
We recommend implementing a slippage check, which essentially does not allow a price to be used should it have been updated more than x blocks ago.
The finding has been acknowledged by the Multisig Labs team. Their official reply is reproduced below:
The price of GGP is used in the Protocol to determine collateralization ratios for minipools as well as slashing amounts. If the price of GGP is unknown or out- dated, the protocol cannot operate. So our remediation for this will be to have a distributed set of Watchers that will Pause the Protocol if the GGP Price becomes outdated. At some point in the future the Protocol will use on-chain TWAP price oracles to set the GGP price.
- Target : MinipoolManager
- Category : Business Logic
- Likelihood : Low
- Severity : Low
- Impact : Low
Due to the nature of the protocol, some fields are queried and used in one intermediary state of the application and then reset in the last state of the application. As an example, see the
avaxNodeOpRewardAmt value, which is queried and used in withdrawMinipoolFunds (which can only be called in the WITHDRAWABLE stage)function withdrawMinipoolFunds(address nodeID) external nonReentrant {
int256 minipoolIndex = requireValidMinipool(nodeID);
address owner = onlyOwner(minipoolIndex);
requireValidStateTransition(minipoolIndex, MinipoolStatus.Finished);
setUint(keccak256(abi.encodePacked(“minipool.item”, minipoolIndex, “.status”)), uint256(MinipoolStatus.Finished));
uint256 avaxNodeOpAmt = getUint(keccak256(abi.encodePacked(“minipool.item”, minipoolIndex, “.avaxNodeOpAmt”)));
uint256 avaxNodeOpRewardAmt = getUint(keccak256(abi.encodePacked(“minipool.item”, minipoolIndex, “.avaxNodeOpRewardAmt”)));
uint256 totalAvaxAmt = avaxNodeOpAmt + avaxNodeOpRewardAmt;
Staking staking = Staking(getContractAddress(“Staking”));
staking.decreaseAVAXStake(owner, avaxNodeOpAmt);
Vault vault = Vault(getContractAddress(“Vault”));
vault.withdrawAVAX(totalAvaxAmt);
owner.safeTransferETH(totalAvaxAmt);
}
and then either reset in the
recordStakingEnd function, to the new rounds’ avaxNodeOpRewardAmt, or set to 0 in recordStakingError.The protocol’s structure assumes that the way in which the states are transitioned through is consistent.
Should major changes occur in the future of the protocol,we suspect that some states that are presumably reset in an eventual state of the protocol may be omitted. This could in turn lead to unexpected consequences to the management of the minipool.
We highly recommend that once important storage states are used, they should also be reset. In this way, future versions of the protocol will have a solid way of transitioning without requiring additional synchronization of storage state.
The issue has ben acknowledged by the Multisig Labs. Their official reply is reproduced below:
The Protocol maintains some fields inStorageso that data such asavaxNodeOpRewardAmtcan be displayed to the end user. The fields will be reset if the user relaunches a minipool with the samenodeIDagain in the future. This is by design.
- Target : Vault.sol
- Category : Business Logic
- Likelihood : Medium
- Severity : Low
- Impact : Informational
Multiple functions from the
Vault contract allow arbitrary tokens to be deposited and withdrawn by networkRegistered contracts. For example, see the depositToken function:function depositToken(string memory networkContractName, ERC20 tokenContract, uint256 amount) external guardianOrRegisteredContracts {
// Valid Amount?
if (amount == 0 ) {
revert InvalidAmount();
}
// Make sure the network contract is valid (will revert if not)
getContractAddress(networkContractName);
// Get contract key
bytes32 contractKey = keccak256(abi.encodePacked(networkContractName, address(tokenContract)));
// Emit token transfer event
emit TokenDeposited(contractKey, address(tokenContract), amount);
// Send tokens to this address now, safeTransfer will revert if it fails
tokenContract.safeTransferFrom(msg.sender, address(this), amount);
// Update balances
tokenBalances[contractKey] = tokenBalances[contractKey] + amount;
}
As per the current implementation, there are no security implications. However, we consider that the
Vault plays an essential role in the entire protocol, and thus we highly recommend fixing this issue for posterity.Upon discussions with the Multisig Lab team, we settled that the best mitigation is
whitelisting the tokenContract that are used in each function. This further allows flexibility and security in smoothly upgrading the Vault should it support more tokens. In that case, the mitigated version of the function could be:function depositToken(string memory networkContractName, ERC20 tokenContract, uint256 amount) external guardianOrRegisteredContracts {
require(whitelisted[tokenContract], “tokenContract not whitelisted”);
if (amount == 0 ) {
revert InvalidAmount();
}
// ...
The issue has been fixed by Multisig Labs in commit 644e8e.
The purpose of this section is to document miscellaneous observations that we made during the assessment.
The
rewardsCycleEnd value from the TokenggAVAX contract should always be evenly divisible by rewardsCycleLength. This condition, however, is only met during the contract initialization, where the rewardsCycleLeng this initially calculated. The rewardsCycleLength is eventually recalculated inside the syncRewards function, but this time, there is no check whether the value is evenly divisible or not.The issue has been fixed by Multisig Labs in commit 556ac4.
- 1.The
calculateAndDistributeRewardsfunction from theClaimNodeOpcontract does not explicitly verify that thestakerAddris a valid staker address. - 2.Add a check that
rewardsPool.getRewardsCycleCount()is not zero to thecalculateAndDistributeRewardsfunction from theClaimNodeOpcontract. - 3.The
registerMultisigfunction in theMultisigManagercontract does not check that themultisig.countvalue has reached 10 to ensure thatThere will never be more than 10 total multisigs, which is a comment on therequireNextActiveMultisigfunction. - 4.The
recordStakingStartfunction in theMinipoolManagercontract does not validate that thestartTimevalue is not greater than the current time. - 5.The set
RewardsStartTimefunction in theStakingcontract does not validate that thetimevalue is not greater than the current time or that it can be only the current time or 0. - 6.The
getInflationAmtin theRewardsPoolcontract does not process the case when the max amount of tokens are released (22_500_000, the total minted amount).
The issue has been fixed by Multisig Labs in commit 878b2e.
In order to receive a reward the staker must be registered for the required amount of time. But the current implementation of the protocol allows users to stake most of the funds immediately before distribution of the reward. The
isEligible function verifies that the staker should be registered at least ProtocolDAO. RewardsEligibilityMinSeconds amount of seconds before the rewards cycle starts (this happens after the first minipool is created), but this check takes into account only the first staking, and the first staked amount may be minimal. Therefore, users can use this possibility to their advantage.The discussion point has been acknowledged by the Multisig Labs team. Their official reply is reproduced below:
We acknowledge that this attack is possible and is a side effect of the nature of our rewards protocol and the short duration of validating on Avalanche. There is some cost and difficulty to exploiting this. It depends on one getting a large amount of GGP before a rewards cycle. If GGP is only available on one AMM, this would greatly move the price with no CEX to arbitrage against. The end result would most likely not be profitable to the attacker if their intention was to dump.
We recommend following the checks-effects-interactions pattern during the
claimAndRestake function in the ClaimNodeOp contract by moving the staking.decreaseGGPRewards(msg.sender, ggpRewards); line above the external calls.The issue has been fixed by Multisig Labs in commit 750812.
In
MinipoolManager the _cancelMinipoolAndReturnFunds function should reset the rewardsStartTime if the .minipoolCount value for staker is zero.The discussion point has been acknowledged by the Multisig Labs team. Their official reply is reproduced below:
We don’t think that resettingrewardsStartTimeis the fix because of the scenario below.
AVAXAssignedHighWatermark, so the AVAX used for this cancelled minipool doesn’t count. Instead we remediated by splitting upavaxAssignedHighWaterandavaxAssignedin this PR. Now the AVAX value used for rewards (avaxAssignedHighWater), will only be increased when the node is started inrecordStakingStart.
The issue has been remediated by Multisig Labs in PR 181.
In
Storage, the intStorage and bytesStorage mappings and related functions are not used and can be deleted.The issue has ben acknowledged by the Multisig Labs and they plan to use them in the future.
We recommend paying additional attention when upgrading the contracts. Should the same
Storage be used, the contract itself might not be re-initializable since its storage would already be used by the previously initialized contract. For example, this could happen in the RewardsPool contract.
function initialize() external onlyGuardian {
if(getBool(keccak256(“RewardsPool.initialized”))) {
Notice that the
RewardPool.initialized will always be true after the first contract has been initialized.The issue has ben acknowledged by the Multisig Labs. Their official reply is reproduced below:
This is by-design. This specific contract was built to ensure even if upgraded that theInflationIntervalStartTimeandRewardsCycleStartTimevalues would not be overwritten.
In the
withdrawAVAX function from Vault, it is assumed that msg.sender has inherited the IWithdrawer interface. We consider that there could be a check for this during the registration process, since in Vault, for example, withdrawAVAX cannot be used (it will revert) unless msg.sender has the IWithdrawer interface implemented beforehand.The discussion point has been acknowledged by the Multisig Labs team. Their official reply is reproduced below:
We added methods to register, unregister and upgrade contracts to the Protocol Dao. We’ll add a check to our deploy scripts to handle verifying that we inherit from IWithdrawer.
In protocol DAO, setters that deal with rates should range from 0.0 - 1.0 ether. This is not directly enforced as of now. The same could be done for the rest of the setter functions in the contract.
The issue has been fixed by Multisig Labs in commit f49931.
In the
startRewardsCycle, the allotment each party is supposed to receive is calculated; however, due to the nature of the arithmetics, some tokens might be left out due to rounding errors.The issue has ben acknowledged by the Multisig Labs and they have determined that the amounts would not be significant.
The purpose of this section is to provide a full threat model description for each function.
As time permitted, we analyzed each function in the smart contracts and created a written threat model for some critical functions. A threat model documents a given function’s externally controllable inputs and how an attacker could leverage each input to cause harm.
Intended behavior:
- Should initialize all state variables and function calls required for the contract to function.
Branches and code coverage:
Intended branches:
- Should be callable by anyone?
- Test coverage
- Should be called after every upgrade.
- Test coverage
Negative behavior:
- Shouldn’t allow 2 x calling this.
- Negative test?
Preconditions:
- Assumes it’s not callable by anyone, or that there’s no way someone can frontrun this transaction
- Assumes that the
Storageis adequately configured (should be fine, sinceguardianrole is assigned in the constructor, for themsg.sender)
Inputs:
asset:- Control : full control
- Checks : no checks
- Impact : used as underlying asset for the vault
storageAddress:- Control : full control
- Checks : no checks
- Impact : used as upgradeable storage contract.
Intended behavior:
This function is used for receiving native tokens. It can be called only by the
asset address.Branches and code coverage:
Intended branches:
- Allow
assetcontract to send native tokens to contract.- Test coverage
Negative behavior:
- It cannot be called from any other address.
- Negative test?
Preconditions:
- the
assetshould be set afterinitializecall
Inputs:
msg.value:- Control : controllable
- Authorization : no
- Impact : -
msg.sender:- Control : controllable
- Authorization :
assert(msg.sender == address(asset)); - Impact :
only accept AVAX via fallback from the WAVAX contract. Otherwise, the balance information may be out of sync.
External call analysis
There are no external calls here.
Intended behavior:
- Should “distribute rewards” to TokenggAVAX holders. Anyone may call this.
lastSync - time of last successful call to this functionrewardsCycleEnd - the time when the total reward will be available;totalReleasedAssets - the full amount of available tokens for withdrawal + the last reward value from the previous cycle. If the reward was not withdrawn immediately after the end of the cycle when the function syncRewards is called for the next cycle, lastRewardsAmt value will be added to the value totalReleasedAssets, and this reward still will be available for withdrawal.Branches and code coverage:
Intended branches:
rewardsCycleEnd= deadline for nextrewardsCycle- Test coverage
lastSync= current timestamp- Test coverage
lastRewardsAmt\_= to the amount that rewards will deplete from.- Test coverage
totalReleasedAssetsis calculated correctly for the next cycle - not sure that it is calculated correctly because it happens differently duringinitializecall- Test coverage
lastRewardsAmtis calculated for the next cycle if the new reward was deposited.- Test coverage
- if rewards didn’t deposit, the
lastRewardsAmtwill equal 0 for the next cycle- Test coverage
lastRewardsAmtis calculated correctly and equals 0 for the first cycle- Test coverage
- if nothing changed since the past cycle
lastRewardsAmtis calculated correctly and equals 0 andtotalReleasedAssetswas increased by the previouslastRewardsAmt- Test coverage
- current
block.timestampshould be less thanrewardsCycleEnd- Test coverage
Negative behavior:
- It basically shouldn’t update unless stuff unless it’s really time to update stuff (see below)
- Negative test?
- Shouldn’t allow calling unless the
rewardsCyclehas passed theblock.timestamp.- Negative test?
Preconditions:
- Assumes that the state variables (
lastRewardsAmt,lastSync,rewardsCycleEndandtotalReleasedAssetsare properly updated) - Can be called by anyone.
Inputs:
Function call analysis
asset.balanceOf(address(this))- What is controllable? The amount of returned value
- If return value controllable, how is it used and how can it go wrong? It can grow if the asset is artificially pumped in the contract;
- What happens if it reverts, reenters, or does other unusual control flow? Doesn’t revert.
Intended behavior:
- This function returns the total amount of underlying assets held by the vault.
Branches and code coverage:
Intended branches:
- After the current cycle ends and the new one starts, the
totalAssetsamount will contain the pastlastRewardsAmtvalue.- Test coverage
totalAssetsis calculated correctly if the current cycle is going.- Test coverage
- If the current cycle ends and the new one doesn’t start, the
totalAssetsshould be equaltotalReleasedAssets\_+lastRewardsAmt- Test coverage
Negative behavior: