feat(protocol): l1 and l2 implementation of Lido integration #17632

swarna1101 · 2024-06-20T12:01:29Z

This PR involves work regarding Taiko ~ Lido WstEth Bridge.

This PR includes L1 contracts implementation, which are responsible for handling actions like deposit, L1 -> L2, communication, and withdrawals, bridge pausing mechanisms etc.
Tests (incomplete)
This PR also contains the L2 contracts implementation

openzeppelin-code · 2024-06-20T12:12:57Z

feat(protocol): l1 and l2 implementation of Lido integration

Generated at commit: 72f2fa9b5b38ca355ed40c57e2376b7dfd44006f

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 2 0 9 42 55
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

adaki2004 · 2024-06-20T12:56:34Z

First, i'd have several remarks:

I'm not sure if this is the right place for the files. Both file-system level in the repo (packages/protocol), or if this is the suitable repo itself, given it runs under GPL-3.0 license, while we have MIT. (Also pointed out by OpenZeppelin previously in their audit)
I dont really get which is the code copied over and which is custom made by @swarna1101 ? I guess all of the solidity code is a take-over, in which case it is not at all final, since we need to adjust/tailor it to our Bridge.
Would be good to have a concept, (a tiny/ugly diagram in excalidraw or paint would be enough) where the working mechanics of the whole bridging procedure is visible

The contracts need to be hooked into our Bridging system somehow. So "something" has to call the Bridge's sendMessage function.

dantaik · 2024-06-20T13:49:30Z

packages/protocol/contracts/thirdparty/lido/.gitmodules

@@ -0,0 +1,6 @@
+[submodule "lib/forge-std"]
+	path = lib/forge-std
+	url = https://github.com/foundry-rs/forge-std


we have modules defined in packages/protocol, do not make another sub-foundry project inside an existing one.

packages/protocol/contracts/thirdparty/lido/src/BridgeableTokens.sol

packages/protocol/contracts/thirdparty/lido/src/L1Executor.sol

dantaik · 2024-06-21T10:56:58Z

First, i'd have several remarks:

I'm not sure if this is the right place for the files. Both file-system level in the repo (packages/protocol), or if this is the suitable repo itself, given it runs under GPL-3.0 license, while we have MIT. (Also pointed out by OpenZeppelin previously in their audit)

I dont really get which is the code copied over and which is custom made by @swarna1101 ? I guess all of the solidity code is a take-over, in which case it is not at all final, since we need to adjust/tailor it to our Bridge.

Would be good to have a concept, (a tiny/ugly diagram in excalidraw or paint would be enough) where the working mechanics of the whole bridging procedure is visible

The contracts need to be hooked into our Bridging system somehow. So "something" has to call the Bridge's sendMessage function.

Given the GPL license, maybe adding a sub-project makes more sense. And we do need to mark our code to be MIT.

dantaik · 2024-06-24T11:29:48Z

Any update?

swarna1101 · 2024-06-24T11:35:01Z

Any update?

yes, I understood to make it compatible to our bridge, in the following commit, it will be:

Compatible bridge
Flow chart
Minor Fixes with proper licensing

dantaik · 2024-06-25T02:02:25Z

Any update?

yes, I understood to make it compatible to our bridge, in the following commit, it will be:

Compatible bridge

Flow chart

Minor Fixes with proper licensing

Ping me when this PR is ready. But I suggest we get this reviewed and approved by the end of this week.

swarna1101 · 2024-06-28T10:24:50Z

Any update?

I have updated the contract as per the Bridge Implementation and also, made sure it is compatible with our bridge.

@adaki2004 added a flow chart as well

L2 implementation is also added now

adaki2004 · 2024-06-28T11:47:19Z

It seems it is a custom Bridge architecture on the top of our Taiko native Bridge's cross-chain messaging - (which is good) - but i still lacking the bit of the context. For example i mean:

Who implemented which contract exactly ? Which is a complete take-over, 100% (which might not require audit) and which is developped completely by you ?
This is more of a component-kinda diagram than a flow chart. Would be cool to have a flow/sequence diagram to get to know the exact workflow (in order) to know what users and also reviews need to look and how.
I see no unit tests - which would be very helpful also to get the context + flow a bit more.

If you could provide some answers to the first 2 points - that would definitely shorten my learning curve and can help a bit more.

swarna1101 · 2024-06-28T13:02:06Z

It seems it is a custom Bridge architecture on the top of our Taiko native Bridge's cross-chain messaging - (which is good) - but i still lacking the bit of the context. For example i mean:

Who implemented which contract exactly ? Which is a complete take-over, 100% (which might not require audit) and which is developped completely by you ?

This is more of a component-kinda diagram than a flow chart. Would be cool to have a flow/sequence diagram to get to know the exact workflow (in order) to know what users and also reviews need to look and how.

I see no unit tests - which would be very helpful also to get the context + flow a bit more.

If you could provide some answers to the first 2 points - that would definitely shorten my learning curve and can help a bit more.

All the contracts in packages/protocol/contracts/thirdparty/lido is taken completely from the existing Lido implementation, and all the contracts in packages/protocol/contracts/partners/lido is implemented by me.
this is the deposit flow , this is Withdram Flow , and this is fail msg .
Yes, unit tests, I will add in next

dantaik · 2024-07-02T03:56:07Z

packages/protocol/contracts/partners/lido/LidoBridgedToken.sol

+     * @param to The address from which the tokens will be burned
+     * @param amount The number of tokens to be burned
+     */
+    function bridgeBurn(address to, uint256 amount) external onlyBridge {


Suggested change

function bridgeBurn(address to, uint256 amount) external onlyBridge {

function bridgeBurn(address from, uint256 amount) external onlyBridge {

dantaik · 2024-07-02T03:56:32Z

packages/protocol/contracts/partners/lido/LidoBridgedToken.sol

+        string memory symbol_,
+        address l2bridge_
+    )
+        ERC20(name_, symbol_)


lido's token decimal is 18?

Lidos token decimals on L2 is 18.

dantaik · 2024-07-02T03:57:43Z

packages/protocol/contracts/partners/lido/LidoBridgedToken.sol

+        _mint(to, amount);
+    }
+
+    /**


You can use /// @inheritdoc ITaiILidoBridgedToken

dantaik · 2024-07-02T03:59:12Z

packages/protocol/contracts/partners/lido/ILidoBridgedToken.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.24;
+
+import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";


maybe put all interfaces inside lido/interfaces

dantaik · 2024-07-02T04:01:18Z

packages/protocol/contracts/thirdparty/lido/BridgeableTokens.sol

+/// @author psirex
+/// @notice Contains the logic for validation of tokens used in the bridging process
+/// @dev Taken from https://github.com/lidofinance/lido-l2/blob/main/contracts/BridgeableTokens.sol
+contract BridgeableTokens {


So all these contracts are not upgradable?

Currently not upgradable. Will make them upgradable

dantaik · 2024-07-02T04:02:54Z

packages/protocol/contracts/partners/lido/LidoL1Bridge.sol

+     * (`false`) the contract.
+     * @param amount_ The amount of tokens being transferred.
+     */
+    modifier transferL1Tokens(bool isTo, uint256 amount_) {


if the token is any arbitrary ERC20 token, then we need this diff-check, if we know it's a specific token implementaiton, we don't.

Is Lido token on L1 a regular ERC20 token?

Yes regular erc20. Will remove check

dantaik · 2024-07-02T04:08:45Z

packages/protocol/contracts/partners/lido/ILidoL1Bridge.sol

+     * @param _message The message received from the L2 bridge
+     * @param _proof The proof of the message
+     */
+    function receiveMessage(IBridge.Message calldata _message, bytes calldata _proof) external;


You have to implement the IMessageInvocable interface in IBridge.sol

You will also need to check permission inside the onMessageInvocation function, please checkout checkProcessMessageContext in ERC20Vault.

dantaik · 2024-07-02T04:10:24Z

packages/protocol/contracts/partners/lido/ILidoL2Bridge.sol

+     * @notice Handles a failed message
+     * @param _message The failed message received
+     */
+    function handleFailMessage(IBridge.Message calldata _message) external;


You probably want to implement IRecallableSender.onMessageRecalled.

dantaik · 2024-07-02T04:11:56Z

packages/protocol/contracts/partners/lido/LidoL1Bridge.sol

+     * @param _message The failed message
+     * @param amount_to_receive The amount of tokens to be received as compensation
+     */
+    function handleFailMessage(


this function must be onMessageRecalled

feat: L1 implementation of Lido integration

ac4763e

swarna1101 requested review from adaki2004 and dantaik June 20, 2024 12:02

swarna1101 changed the title ~~feat: L1 implementation of Lido integration~~ feat (protocol): L1 implementation of Lido integration Jun 20, 2024

swarna1101 changed the title ~~feat (protocol): L1 implementation of Lido integration~~ feat(protocol): L1 implementation of Lido integration Jun 20, 2024

swarna1101 changed the title ~~feat(protocol): L1 implementation of Lido integration~~ feat(protocol): l1 implementation of Lido integration Jun 20, 2024

dantaik reviewed Jun 20, 2024

View reviewed changes

packages/protocol/contracts/thirdparty/lido/src/BridgeableTokens.sol Outdated Show resolved Hide resolved

dantaik reviewed Jun 20, 2024

View reviewed changes

packages/protocol/contracts/thirdparty/lido/src/BridgeableTokens.sol Outdated Show resolved Hide resolved

dantaik reviewed Jun 20, 2024

View reviewed changes

packages/protocol/contracts/thirdparty/lido/src/L1Executor.sol Outdated Show resolved Hide resolved

fix: l1 implementation of Lido integration

c670fd3

swarna1101 and others added 4 commits June 28, 2024 15:56

fix: minor changes

1876806

fix: minor edge case

97c69e0

feat: change in IBridge

8020a37

forge fmt & update contract layout table

69e757f

swarna1101 changed the title ~~feat(protocol): l1 implementation of Lido integration~~ feat(protocol): l1 and l2 implementation of Lido integration Jun 28, 2024

Merge branch 'main' into lido-integration

7f2d101

fix: license version

c220304

davidtaikocha added the area.protocol label Jun 29, 2024

Merge branch 'main' into lido-integration

5597cf2

dantaik requested changes Jul 2, 2024

View reviewed changes

Merge branch 'main' into lido-integration

72f2fa9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(protocol): l1 and l2 implementation of Lido integration #17632

feat(protocol): l1 and l2 implementation of Lido integration #17632

swarna1101 commented Jun 20, 2024 •

edited

Loading

openzeppelin-code bot commented Jun 20, 2024 •

edited

Loading

adaki2004 commented Jun 20, 2024

dantaik Jun 20, 2024

dantaik commented Jun 21, 2024

dantaik commented Jun 24, 2024

swarna1101 commented Jun 24, 2024

dantaik commented Jun 25, 2024

swarna1101 commented Jun 28, 2024 •

edited

Loading

adaki2004 commented Jun 28, 2024

swarna1101 commented Jun 28, 2024 •

edited

Loading

dantaik Jul 2, 2024

dantaik Jul 2, 2024

swarna1101 Jul 2, 2024

dantaik Jul 2, 2024

dantaik Jul 2, 2024

dantaik Jul 2, 2024

swarna1101 Jul 2, 2024

dantaik Jul 2, 2024

swarna1101 Jul 2, 2024

dantaik Jul 2, 2024

dantaik Jul 2, 2024

dantaik Jul 2, 2024

dantaik Jul 2, 2024

	function bridgeBurn(address to, uint256 amount) external onlyBridge {
	function bridgeBurn(address from, uint256 amount) external onlyBridge {

feat(protocol): l1 and l2 implementation of Lido integration #17632

Are you sure you want to change the base?

feat(protocol): l1 and l2 implementation of Lido integration #17632

Conversation

swarna1101 commented Jun 20, 2024 • edited Loading

openzeppelin-code bot commented Jun 20, 2024 • edited Loading

feat(protocol): l1 and l2 implementation of Lido integration

🚨 Report Summary

adaki2004 commented Jun 20, 2024

Choose a reason for hiding this comment

dantaik commented Jun 21, 2024

dantaik commented Jun 24, 2024

swarna1101 commented Jun 24, 2024

dantaik commented Jun 25, 2024

swarna1101 commented Jun 28, 2024 • edited Loading

adaki2004 commented Jun 28, 2024

swarna1101 commented Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swarna1101 commented Jun 20, 2024 •

edited

Loading

openzeppelin-code bot commented Jun 20, 2024 •

edited

Loading

swarna1101 commented Jun 28, 2024 •

edited

Loading

swarna1101 commented Jun 28, 2024 •

edited

Loading