Report for issue #864 updated by MrPotatoMagic

code-423n4 · Oct 6, 2023 · 3e4b9d6 · 3e4b9d6
1 parent aa6e10f
commit 3e4b9d6
Showing 1 changed file with 5 additions and 3 deletions.
diff --git a/data/MrPotatoMagic-Analysis.md b/data/MrPotatoMagic-Analysis.md
@@ -62,7 +62,7 @@ Day 11-14
 
 The protocol has been structured in a very systematic and ordered manner, which makes it easy to understand how the function calls propagate throughout the system. 
 
-With LayerZero at the core of the system, the UA entry/exit points are the Bridge Agents which further interact with LayerZero in the order Bridge Agent => Endpoint => ULN messaging library. Check out [Mechanism Review](#mechanism-review) for a hihg-level mental model.
+With LayerZero at the core of the system, the UA entry/exit points are the Bridge Agents which further interact with LayerZero in the order Bridge Agent => Endpoint => ULN messaging library. Check out [Mechanism Review](#mechanism-review) for a high-level mental model.
 
 Although the team has provided sample routers that users can customize to their needs, my only recommendation is to provide more documentation or a guide to the users on how custom Routers can be implemented while ensuring best encoding/decoding practices when using abi.encode and abi.encodePacked. Providing such an outstanding hard-to-break complex protocol is a plus but it can be a double-edged sword if users do not know how to use it while avoiding bugs that may arise.
 
@@ -79,7 +79,7 @@ Although the team has provided sample routers that users can customize to their
 
 1. Maia VS [Altitude DEFI](https://altitude-1.gitbook.io/altitude/) - Similar to Maia, [Altitude DEFI](https://www.altitudedefi.com/transfer) is a cross-chain protocol that uses LayerZero. But they both do have their differences. First, Maia allows users to permisionlessly add ERC20 tokens to the protocol but Altitude DEFI does not. Second, Maia identifies token pairs using chainIds and local, global and underlying tokens stored in Port mappings but Altitude DEFI uses an ID system for token pairs that are identified through their Pool IDs ([More about Altitude Pool Ids here](https://altitude-1.gitbook.io/altitude/ecosystem/pool-ids)). Maia has a much better model overall if we look at the above differences. I would still recommend the sponsors to go through [Altitude's documentation](https://altitude-1.gitbook.io/altitude/) in case any more value can be extracted from a protocol that is already live onchain.
 
-2. Maia VS [Stargate Finance](https://stargate.finance/transfer) - Similar to Maia, [Stargate Finance](https://stargate.finance/transfer) is a cross-chain protocol that uses LayerZero. There is a lot to gain from this project since it is probably the best ([audited 3 times](https://stargateprotocol.gitbook.io/stargate/audits)) live bridging protocol on LayerZero. Let's understand the differences. Similar to Altitude DEFI, Stargate uses Pool IDs to identify token pairs while Maia identifies token pairs using chainIds and local, global and underlying tokens stored in Port mappings. Secondly, Stargate has implemented an [EQ Gas Fee projection tool](https://stargateprotocol.gitbook.io/stargate/developers/eq-fee-projection) that provides fee estimates of different chains on [their frontend](https://stargate.finance/transfer). Maia has implemented the function [getFeeEstimate()](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L161) function but it does not provide Branch-to-Branch estimates. For example, when bridging from Branch Bridge Agent (on AVAX) to another Branch Bridge Agent (on Fantom) through the RootBridgeAgent, the [getFeeEstimate()](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L161) function in BranchBridgeAgent (on AVAX) only allows estimation of fees for the call from Branch chain (AVAX) to Root chain (Arbitrum) and not the ultimate call from AVAX to Fantom. This is because the [dstChainId field is hardcoded with rootChainId](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L167). This is a potential shortcoming of the estimator since it does not calculate the fees for the whole exection path (in our case fees from Arbitrum to Fantom). Maia has the potential to be at or above par with Stargate Finance, thus I would recommend the sponsors to go through [their documentation](https://stargateprotocol.gitbook.io/stargate/) in case any further protocol design weak spots or features could be identified.
+2. Maia VS [Stargate Finance](https://stargate.finance/transfer) - Similar to Maia, [Stargate Finance](https://stargate.finance/transfer) is a cross-chain protocol that uses LayerZero. There is a lot to gain from this project since it is probably the best ([audited 3 times](https://stargateprotocol.gitbook.io/stargate/audits)) live bridging protocol on LayerZero. Let's understand the differences. Similar to Altitude DEFI, Stargate uses Pool IDs to identify token pairs while Maia identifies token pairs using chainIds and local, global and underlying tokens stored in Port mappings. Secondly, Stargate has implemented an [EQ Gas Fee projection tool](https://stargateprotocol.gitbook.io/stargate/developers/eq-fee-projection) that provides fee estimates of different chains on [their frontend](https://stargate.finance/transfer). Maia has implemented the [getFeeEstimate()](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L161) function but it does not provide Branch-to-Branch estimates. For example, when bridging from Branch Bridge Agent (on AVAX) to another Branch Bridge Agent (on Fantom) through the RootBridgeAgent, the [getFeeEstimate()](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L161) function in BranchBridgeAgent (on AVAX) only allows estimation of fees for the call from Branch chain (AVAX) to Root chain (Arbitrum) and not the ultimate call from AVAX to Fantom. This is because the [dstChainId field is hardcoded with rootChainId](https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L167). This is a potential shortcoming of the estimator since it does not calculate the fees for the whole exection path (in our case fees from Arbitrum to Fantom). Maia has the potential to be at or above par with Stargate Finance, thus I would recommend the sponsors to go through [their documentation](https://stargateprotocol.gitbook.io/stargate/) in case any further protocol design weak spots or features could be identified.
 
 There are more protocols that use LayerZero such as Across and Synapse. Each of them have similar implementations and small differences. I would recommend the sponsors to go through their documentations in case any value could further be extracted from them.
 
@@ -213,7 +213,7 @@ The below contracts are Singluar/Helper contracts that do not inherit from any i
 
  - Configuring with incorrect chainId possibility - Currently the LayerZero propritary chainIds are constants (as shown on their website) and are highly unlikley to change. Thus when pasing them as input in the constructor, they can be compared with hardcoded magic values (chainIds) to decrease chances of setting a wrong chainId for a given chain.
  - Ensuring safer migration - In case of migration to a new LayerZero messaging library, it is important to provide the users and members of the Maia community with a heads up announcement in case there are any pending failed deposits/settlements left to retry, retrieve or redeem.
- - Filtering poison ERC20 tokens - Filtering out poison ERC20 tokens from the user frontend is important. Warnings should be provided before cross-chain interactions with a non-verified or unnamed or non-reputed token is being made.
+ - Filtering poison ERC20 tokens - Filtering out poison ERC20 tokens from the user frontend is important. Warnings should be provided before cross-chain interactions when a non-verified or unnamed or non-reputed token is being used.
  - Hardcoding _payInZRO to false - Although there are several integration issues (included in QA report), the team should not hardcode the `_payInZRO` field to false since according to the LayerZero team it will make future upgrades or adapting to new changes difficult for the current bridge agents (Mostly prevent the bridge agent from using a future ZRO token as a fee payment option). Check out [this transcript](https://tickettool.xyz/direct?url=https://cdn.discordapp.com/attachments/1155911737397223496/1155911741725757531/transcript-tx-mrpotatomagic.html) or the image below for a small conversation between 10xkelly and I on this subject
 
 ![Imgur](https://i.imgur.com/2AFJWLg.png)
@@ -259,5 +259,7 @@ Is it possible for branch bridge agent factories to derail the message from its
 Is the encoding/decoding with abi.encode, abi.encodePacked and abi.decode done correctly?
  - Yes, the safe encoding/decoding in the Maia protocol is one of the strongest aspect that makes it bug-proof. 
 
+
+
 ### Time spent:
 150 hours