Bug: Tokens May Not Be Added To Whitelist on Client Chains #80
Assignees
Comments
adu-web3
changed the title
MaxMustermann2
added a commit
to MaxMustermann2/exocore-contracts
that referenced
this issue
Aug 31, 2024
ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes ExocoreNetwork#80
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 5, 2024
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 14, 2024
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments * feat(tvl-limit): impose tvl limit on client chain We can consider the TVL limits to be of two types: one is the logical TVL limit which disallows deposits from exceeding total supply and the other is the operational TVL limit which is used for risk management during the launch phase of the network. The former is imposed by Exocore's precompiles while the latter is imposed by the client chain's gateway contracts. Both fields are editable by the contract owners: the total supply in response to minting and burning and the deposit limits with time to larger values. As a consequence of the introduction of a limit on the client chain, it must now be provided at the time of token registration. It can be updated or reduced unilaterally on the client chain. While editing the `totalSupply` on Exocore, it is best to be fully accurate. Reducing the `totalSupply` to lower than the TVL limit configured on the client chain will result in failed deposits which may produce a system halt. Even if the TVL limit is reduced on the client chain after this, it can impact the messages that are already in-flight. * fix(doc): update IAssets comment * feat: prevent misconfig of supply and tvl limits The objective of this change is to prevent deposit failures by ensuring that the tvlLimit (as enforced on the client chain) <= totalSupply (as enforced by Exocore). Note that, with time, the totalSupply stored on Exocore may become outdated as well; however, it is a secondary check which occurs only after tokens are transferred into the Vault on the client chain. Since the addition of tokens to the whitelist happens through Exocore, it can be enforced easily that the tvlLimit of said token is less than the total supply (as known to Exocore). Whenever a transaction to update the tvlLimit on a client chain is received, it is handled on a case-by-case basis. - A decrease in tvl limit is always allowed. - An increase in tvl limit is forwarded to Exocore for validation. Exocore receives this validation request, and responds in the affirmative if the new tvl limit <= supply of the token (again, according to Exocore). It responds in the negative, otherwise. Reciprocally, whenever an attempt is made to change the recorded value of total supply corresponding to a token on Exocore, here's what happens. - An increase in the total supply is always allowed. - A decrease in the total supply is forwarded to the client chain for validation. when the client chain receives this validation request, it responds in the affirmative if the tvl limit <= new total supply (which may be totally different from the actual total supply due to changes in the number when the message was in-flight). Otherwise, it responds in the negative. One caveat for both of these validations is that if a validation request arising from the validating chain is already in-flight, the validation is rejected. This is to ensure that in-flight messages cannot result in a misconfiguration. For example, if the TVL limit is increased from 400m to 500m and the total supply is 600m, it will be approved. However, if, at the same time, there is a validation request sent to the client chain by Exocore to verify if the total supply can be reduced to 450m, it could cause inconsistency. In this event, the TVL limit increase is rejected and the total supply decrease will be approved. * test: validate nonces once done * feat(bootstrap): reject if tvl limit > supply * fix: use a count of in-flight messages, not bool * fix: native restake limit + non whitelist check * fix: index requests by chainId and nonce Mistakenly, only the nonce was used but that is not accurate * fix: simplify TVL limit and total supply limit Based on the discussion, it was decided to remove the total supply limit enforced by Exocore. Instead, the TVL limit will be imposed on the client chain, which will also enforce that any transfers more than the total supply will fail. It is possible to set a value of infinite TVL limit by using `type(uin256).max` * fix(script): print ProxyAdmin address to JSON * feat(ci): fail compilation if code size is large Within Foundry's configuration, the `deny_warnings` boolean may be set to `true` to force it to report a compilation failure in the case of warnings. With the `code-size` warning enabled, this will cause a build failure and report to the developer. * fix: remove unused variables * fix: remove some more unused fields * feat: use `uint128` for tvl limit on exo gateway
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
The function addOrUpdateWhitelistTokens adds or updates tokens and if a token was added, it sends a REQUEST_ADD_WHITELIST_TOKENS message to a client chain.
However, the updated variable corresponds to the last token in the tokens array, meaning that if a token earlier in the array was added and the last token in the array was only updated, the REQUEST_ADD_WHITELIST_TOKENS will not be sent.
Note that calling addOrUpdateWhitelistTokens again with the recently added token will not resolve the issue as that will update the token, so the !updated value will be false and the REQUEST_ADD_WHITELIST_TOKENS message will not be sent.
reference: https://skyharbor.certik.com/report/04d25ca5-49da-4609-99cf-e59242a8914e?findingIndex=EGE-01
The text was updated successfully, but these errors were encountered: