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

feat(gwyneth): incorporate booster sample contracts #26

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

adaki2004
Copy link

There are some questions ariesed, i tried to asked them in form of some comments.

Also some libraries/contracts/interfaces like:
IExternalL2, IExternalL2Bridge, DemoReadFromExternalL2 , Create2, IProxy, WalletProxy, SmartWallet, WalletFactory are missing because first wanted to try with a simple erc20.

@adaki2004 adaki2004 requested a review from Brechtpd July 23, 2024 13:22
// Only stored on L1
// Currently getBlockHash() is not supported via the new Taiko Gwyneth
//ITaiko public taiko;
// todo (@Brecht): XChain has a bus property but Bus is an XChain (inherits). It does not make too much sense to me, or maybe i'm missing the point ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bus might still be needed for async transactions to pass data from one chain to the other, but I guess to keep things simple let's assume synchronous everywhere for now.

// These could also be exposed using a precompile because we could get them from public input,
// but that requires extra work so let's just fetch them from L1 for now
function getBlockHash(uint chainID, uint blockID) external view xExecuteOn(EVM.l1ChainId) returns (bytes32) {
// todo(@Brecht): Currently not supported or well, at least TaikoL1 does not have it with the current design.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, outdated! Hmmmm only needed for async case so let's skip for now.

@Brechtpd
Copy link

Let's assume a wondrous fully synchronous world 🌈 . XCALLOPTIONS is able to do any call to any chain, so we can avoid the Bus for now.

@adaki2004
Copy link
Author

Let's assume a wondrous fully synchronous world 🌈 . XCALLOPTIONS is able to do any call to any chain, so we can avoid the Bus for now.

Currently i can move synch-only code (from Bus to XChain), like write() + consume() (async case) but the xFunction modifier is in the xChain by default, and it handles async path too (EVM.chainId() == fromChainId).

Questions:
So basically you saying now, if a message is sync, xTransfer is never called on sourceChain ? If so, i think the balance calculation is not correct in XChainToken. (i guess we need to update balance on both for example).

Would be cool to go thru diff. xTransfer scenarios:

  • L1 -> L2x
  • L2x -> L2y
    (Would be cool to cover asycn + sync scenarios, but maybe sync first, if we assume a synch world)

@adaki2004 adaki2004 merged commit beb7199 into gwyneth Jul 25, 2024
16 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants