EVM Bridge Contracts - Pontis

Users who want to utilize the bridgeOutRunes() function in the pontis-bridge-v1.sol contract, have to pay fees in the native token. First the fetchPrices() function is called which returns the runePrice and nativePrice. Both of those prices are fetched from an oracle implementation:

    function fetchPrices(string calldata runeId) private view returns (uint256 nativePrice, uint256 runePrice) {
        nativePrice = priceOracle.getPrice(NATIVE_TOKEN_NAME);
        runePrice = priceOracle.getPrice(runeId);
        require(nativePrice > 0, "Invalid Native Price");
        require(runePrice > 0, "Invalid Rune Price");
        return (nativePrice, runePrice);
    }

The getPrice() function fetches the price from a specific contract implementation - pontis-price-oracle.sol, where the price is set via the setPrices() function. According to the dev team the price will be updated once a day, this is a problem if the price is too volatile. As this will result in users paying more fees than they should, or the protocol receiving less in fees, depending on how the price fluctuates.

Consider fetching the prices from oracles such as Chainlink or Pyth.

RISK ACCEPTED: The Pontis team accepted the risk of this finding.

The current ownership transfer process for the pontis-bridge-nft.sol , pontis-bridge-fee-manager.sol , pontis-bridge-controller.sol contracts utilizes a direct transfer of ownership, if the owner address is set incorrectly, all of the critical functions of the protocol will be disabled or compromised.

Consider utilizing OpenZeppelin Ownable2Step.sol contract.

PARTIALLY SOLVED: The Pontis team implemented the recommended solution in the pontis-bridge-nft.sol and pontis-bridge-fee-manager.sol contracts. However, they decided not to implement any changes in the pontis-bridge-controller.sol contract.

In Solidity smart contract development, replacing hard-coded revert message strings with the Error() syntax is an optimization strategy that can significantly reduce gas costs. Hard-coded strings, stored on the blockchain, increase the size and cost of deploying and executing contracts.

The Error() syntax allows for the definition of reusable, parameterized custom errors, leading to a more efficient use of storage and reduced gas consumption. This approach not only optimizes gas usage during deployment and interaction with the contract but also enhances code maintainability and readability by providing clearer, context-specific error information.

It is recommended to replace hard-coded revert strings in require statements for custom errors, which can be done following the logic below:

1. Standard require statement (to be replaced):

require(condition, "Condition not met");

2. Declare the error definition to state:

error ConditionNotMet();

3. As currently is not possible to use custom errors in combination with require statements, the standard syntax is:

if (!condition) revert ConditionNotMet();

More information about this topic in the Official Solidity Documentation.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

In the pontis-bridge-v1.sol contract when fees in the native tokens are paid, the solidity built in transfer() function is used to handle the native token transfer, it does this by forwarding a fixed amount of 2300 gas. Paying fees in the native token is function which is utilized by all of the core functionalities of the pontis-bridge-v1.sol contract. This is dangerous for two reasons:

1. Gas costs of EVM instructions may change significantly during hard forks, which may previously assumed fixed gas costs. EIP 1884 as an example, broke several existing smart contracts due to a cost increase of the SLOAD instruction.

2. If the recipient is a contract or a multisig safe, with a receive/fallback function which requires >2300 gas, e.g safes that execute extra logic in the receive/fallback function, the transfer function will always fail for them due to out of gas errors.

Consider using the call() method for transferring funds, as it is more flexible and handles the increase in gas cost associated with the transfer. Be careful not to introduce a reentrancy vulnerability.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

In the pontis-bridge-v1.sol contract, the setRunesAndBtcActive() function loops through all of the provided addresses in the runes array. However, the length of the runes array is not cached before all request are looped trough. This increases the gas consumed by the setRunesAndBtcActive() function. The same scenario occurs in the pegOutOrdinalsBatch(), mintOrdinalsBatch(), mintRunesBatch() functions as well.

Consider caching the length of the array, before you loop over it.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

The project is using a Solidity version greater than 0.8.18, which supports named mappings. Using named mappings can improve the readability and maintainability of the code by making the purpose of each mapping clearer. This practice helps developers and auditors understand the mappings' intent more easily.

Consider refactoring the mappings to use named arguments, which will enhance code readability and make the purpose of each mapping more explicit.

For example, in the pontis-bridge-v1.sol contract, instead of declaring:

mapping(address => bool) private runeTokensActive;

It could be declared as:

mapping(address runeTokenAddress => bool active) private runeTokensActive;

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

All contracts in scope currently use floating pragma versions ^0.8.26 which means that the code can be compiled by any compiler version that is greater than or equal to 0.8.26, and less than 0.9.0.

However, it is recommended that contracts should be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

In this aspect, it is crucial to select the appropriate EVM version when it's intended to deploy the contracts on networks other than the Ethereum mainnet, which may not support these opcodes. Failure to do so could lead to unsuccessful contract deployments or transaction execution issues.

Lock the pragma version to the same version used during development and testing. Additionally, make sure to specify the target EVM version when using Solidity versions from 0.8.20 and above if deploying to chains that may not support newly introduced opcodes. Additionally, it is crucial to stay informed about the opcode support of different chains to ensure smooth deployment and compatibility.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

None of the important fields in the events emitted in the pontis-bridge-fee-manager.sol contract and the pontis-bridge-v1.sol contracts are indexed.

Index the appropriate fields in the events that are emitted in the pontis-bridge-fee-manager.sol and pontis-bridge-v1.sol contracts.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

In the pontis-bridge-nft.sol contract, the setBaseURI() function sets a new baseURI, which is used by the getTokenURI() function in order to return a correct URI address. However, no event is emitted when a new baseURI is set.

Emit an event in the setBaseURI() function.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.

In the pontis-bridge-nft.sol contract in the setBaseURI() function, the memory keyword is used for a parameter which is not modified instead of the calldata keyword.

Consider using the calldata keyword instead of the memory for function arguments which are not modified.

SOLVED: The Pontis team has followed the recommendation and successfully resolved the issue.