Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ethereum limit payload size and documentation to run bridge contracts locally #224

Merged
merged 3 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 10 additions & 18 deletions apps/blockchain/README.md
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
# Starklane technical guide
# ArkProject Bridge technical guide

Starklane is a suite of tools for developers to easily bridge asset between Ethereum and Starknet.
ArkProject Bridge is a suite of tools for developers to easily bridge asset between Ethereum and Starknet.

It should not be seen as a very generic tool as Starkgate provides. As the considerations and implications of bridging a NFT are very different.

The current implementation of Starklane is aiming at supporting both [ERC721](https://eips.ethereum.org/EIPS/eip-721) and [ERC1155](https://eips.ethereum.org/EIPS/eip-1155). In the case of ERC1155, there are economics implications of bridging assets that are beyond the scope of this document. The ERC721 is easier to handle due to the inherent uniqueness of a token.
The current implementation of ArkProject Bridge is aiming at supporting both [ERC721](https://eips.ethereum.org/EIPS/eip-721) and [ERC1155](https://eips.ethereum.org/EIPS/eip-1155). In the case of ERC1155, there are economics implications of bridging assets that are beyond the scope of this document. The ERC721 is easier to handle due to the inherent uniqueness of a token.

# Considerations

Working with ERC721 and ERC1155 contracts has different constraints on Ethereum and Starknet. Indeed, Ethereum being more standardized at the moment of this writing, it’s easier to have some assumptions on what a contract must implement/expose using the [ERC165](https://eips.ethereum.org/EIPS/eip-165) specification.

- **ERC721**: The metadata (Token URI most importantly) for a token can be implemented in diverse manners. Using a base URI concatenating the token id, or having a string stored for each token. Even if the base URI is often use, we still see some URIs being stored.
We can see here the first choice to make when a token is bridged. Is the URI required to be explicit bridged for each token, or only bridging the base URI is sufficient.
This is then tied to what is the design choice made by the developer / collection owner. Starklane supports both.
This is then tied to what is the design choice made by the developer / collection owner. ArkProject Bridge supports both.
- **ERC1155**: Metadata for ERC1155 is from the beginning using a base URI. Which is implemented in this same fashion for every contracts. However, with ERC1155, the consideration is about economics and how to keep track of tokens amounts being bridge.
In fact, as token may not be unique - and even unique token are not guaranteed to be unique, the escrow mechanism to hold the tokens when bridged can’t follow the same logic as ERC721 escrow does.
This topic is still under discussion at Screenshot Labs, but the most viable option seems to be a burn of the tokens being bridged, and mint them again if they are bridged from the other chain.
But again, this is more about economics than technical consideration.
- **Cairo version**: Starklane is being updated to be compliant with `v2.4.0`.
- **Cairo version**: ArkProject Bridge is being updated to be compliant with `v2.6.4`.
- **Starknet messaging asymmetry**: The native messaging proposed by Starknet is working differently based on which direction assets are bridged.
L1→L2: From a single transaction on Ethereum, the message is registered and emitted as an event by the Starknet Core Contract, and the sequencer is responsible of gathering those events to then send (automatically) a `L1Handler` transaction to the destination contract on Starknet.
L2→L1: When a transaction is made on Starknet with a message for Ethereum, the message is included in a block (Ethereum is not listening to events). Once this block is proven, Starknet Core Contract is capable of taking this message in account, and at this moment a transaction on Ethereum must be realized to actually consume the message (and thus execute the logic associated to it).
At the moment of this writing, the proving time is quite long for a good UX. For this reason, we propose to developer to use an indexer that is able to fire transactions on Ethereum reacting on the `requests` being included in a block (and emitted in an event). This transaction has more cost, and the developer / collection owner can choose to pay the costs upfront for his users, charge more fees at the moment of the deposit, or simply let the user do the withdraw when the block is proven without firing this transaction.
Important to note that this transaction to “consume the message before the block is being proved” is implemented in a way that even when the block is proven later, the “consume message” proposed by Starknet Core will be invalid. This ensure the tokens can’t be withdrawn two times.
- **Auto-deploy of contracts**: As some collection owner that are bridging may not have deployed a collection on the other chain to receive the token, Starklane is auto-deploying a contract to receive the tokens if no collection address on the destination chain is given in the request. This contract is proxied on Ethereum, and upgradable on Starknet, to ensure that the owner can take back the full control without loosing the data.
Those auto-deployed contract implements the `IStarklaneBridgeable` interface, which ensures that the contract can receive/send token through Starklane.
- **Auto-deploy of contracts**: As some collection owner that are bridging may not have deployed a collection on the other chain to receive the token, ArkProject Bridge is auto-deploying a contract to receive the tokens if no collection address on the destination chain is given in the request. This contract is proxied on Ethereum, and upgradable on Starknet, to ensure that the owner can take back the full control without loosing the data.
Those auto-deployed contract implements the `IStarklaneBridgeable` interface, which ensures that the contract can receive/send token through ArkProject Bridge.
If the collection owner / developer already deployed a contract, this contract must implement `IStarklaneBridgeable` to interact with the bridge.

# Request protocol

Starklane has a protocol designed to work around a `request` to bridge assets. A `request` is self explanatory about the collection and tokens to bridge.
ArkProject Bridge has a protocol designed to work around a `request` to bridge assets. A `request` is self explanatory about the collection and tokens to bridge.

A single `request` can bridge one or more tokens for **ONE** collection. To bridge assets of different collections, several `request` must be made.

Expand Down Expand Up @@ -85,9 +85,9 @@ struct Request {
- Withdraw strategies: When assets are bridge from Ethereum to Starknet, the withdraw is always `auto` as the transaction `L1Handler` is fired by the sequencer.
When transferring assets from Starknet to Ethereum, the withdraw strategies are:
`auto` ⇒ The indexer fires a transaction on Ethereum when the `request` is included in a block produced on starknet (but not yet proven) which triggers the automatic withdraw of the assets on L1.
`manual` ⇒ The withdraw of the assets will be possible only when the block in which the `request` was included is being proven. At this moment, the user must send a transaction to Starklane contract on Ethereum to withdraw the assets.
`manual` ⇒ The withdraw of the assets will be possible only when the block in which the `request` was included is being proven. At this moment, the user must send a transaction to ArkProject Bridge contract on Ethereum to withdraw the assets.
- Deposit strategies:
`escrow` ⇒ The token is held on escrow by Starklane contract. A token held on escrow by Starklane contract may also be burnt by the collection owner (i.e.: When the token is safely bridge, and the collection owner wants to burn the collection after being bridged).
`escrow` ⇒ The token is held on escrow by ArkProject Bridge contract. A token held on escrow by ArkProject Bridge contract may also be burnt by the collection owner (i.e.: When the token is safely bridge, and the collection owner wants to burn the collection after being bridged).
`burn-auto` ⇒ This one uses the same approach as the `withdraw-auto`, when the request is successfully processed (assets are bridged), the indexer fires a transaction on the other chain to burn the tokens. This works for both Ethereum and Starknet, as the messaging is asynchronous.
This is the safest way to automatically handle the burn, even if the cost is a bit higher.
TODO: May we consider separating a `burn` and `burn-auto`? To allow the same transaction to burn then bridge? But it’s very risky, for ERC721 especially as a burnt token can’t be minted again.
Expand All @@ -100,11 +100,3 @@ The hash computation includes only one collection address, the collection addres
You can find a first representation of the FMS on this [figma](https://www.figma.com/file/esIDAZS1UySOAtq7hMa5xQ/FSM-For-Starklane---L1---L2?type=whiteboard&node-id=0-1&t=LqPt9ELDlcVdz29t-0).
We're currently reviewing and we will detail more some steps to ensure a better preparation for the audit and easier for someone to dive into the project.

# TODOs
- [ ] Review linear / github issue.
- [ ] Add forge binary with l2_handler_caller for CI (waiting the PR to be merged).
- [ ] Rework ethereum to optimize size of contrat.
- [ ] Terminate indexer to allow withdraw auto on ethereum + burn auto both side.
- [ ] Check metadata for ERC1155 and ERC721 (how to allow all strategies to be included).
- [ ] Connect UI both sides with new contract interface.
- [ ] Validate new implementation in Katana to submit PR.
8 changes: 7 additions & 1 deletion apps/blockchain/ethereum/.env.anvil
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
# General config.
ETH_RPC_URL=http://127.0.0.1:8545
ETHERSCAN_API_KEY=YGJJHTCQBYE2C336HMUCY5P27MJY885IIN
LOCAL_LOGS=logs/
ETHERSCAN_API_KEY=fake_key

# Account related variables (EOA account).
ACCOUNT_PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
ACCOUNT_ADDRESS=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266

ETHEREUM_USER_ADDR=0x70997970C51812dc3A010C7d01b50e0d17dc79C8
ETHEREUM_USER_PRIVATE_KEY=0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d

# Starklane L1 variables.
STARKNET_CORE_L1_ADDRESS=0x0000000000000000000000000000000000000000
STARKLANE_L1_PROXY_ADDRESS=0x0000000000000000000000000000000000000000
Expand All @@ -16,3 +19,6 @@ STARKLANE_L1_PROXY_ADDRESS=0x0000000000000000000000000000000000000000
STARKLANE_L2_ADDRESS=0x075af152a7ad8c32726093fdf9820de103854d15a8ef4fae11d2739671ff1f9e
STARKLANE_L2_SELECTOR=0x005d25c8970b257e237fddc686e4a2711310774d2277107e1ebaf3e0a68dd037
OWNER_L2_ADDRESS=0x1

alias cast_admin_send="cast send --private-key ${ACCOUNT_PRIVATE_KEY}"
alias cast_user_send="cast send --private-key ${ETHEREUM_USER_PRIVATE_KEY}"
2 changes: 1 addition & 1 deletion apps/blockchain/ethereum/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ anvil = "http://127.0.0.1:8545"

[etherscan]
goerli = { key = "${ETHERSCAN_API_KEY}" }
anvil = { key = "${ETHERSCAN_API_KEY", chainId=31337}
anvil = { key = "FAKE KEY", chainId=31337}
7 changes: 6 additions & 1 deletion apps/blockchain/ethereum/src/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ error NotSupportedYetError();
error CollectionMappingError();
error NotWhiteListedError();
error BridgeNotEnabledError();
error TooManyTokensError();

uint256 constant MAX_PAYLOAD_LENGTH = 300;

/**
@title Starklane bridge contract.
Expand Down Expand Up @@ -128,7 +131,9 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
req.tokenIds = ids;

uint256[] memory payload = Protocol.requestSerialize(req);

if (payload.length >= MAX_PAYLOAD_LENGTH) {
revert TooManyTokensError();
}
IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
Expand Down
19 changes: 0 additions & 19 deletions apps/blockchain/ethereum/src/token/ERC721Bridgeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,25 +48,6 @@ contract ERC721Bridgeable is ERC721, UUPSOwnableProxied, IERC721Bridgeable {
_transferOwnership(_msgSender());
}

/**
@notice A free minting for testing.
*/
function mintRangeFree(
address to,
uint256 idStart,
uint256 idEnd
)
external
{
require(idStart < idEnd, "Bad range");

uint256 id = idStart;

for (id; id <= idEnd; id++) {
_mint(to, id);
}
}

/**
@inheritdoc IERC721Bridgeable

Expand Down
Loading
Loading