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

Update EIP-5792: make atomicBatch chain-specific per-batch, not per-call (both in protocol messages and in normative prose) #8626

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bumblefudge
Copy link
Contributor

As per a conversation with @pedrouid and @jxom

@bumblefudge bumblefudge requested a review from eth-bot as a code owner June 6, 2024 08:55
@github-actions github-actions bot added c-update Modifies an existing proposal t-interface labels Jun 6, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Jun 6, 2024

File EIPS/eip-5792.md

Requires 1 more reviewers from @arein, @drortirosh, @jxom, @lukasrosario, @moodysalem, @wilsoncusack

@eth-bot eth-bot added the a-review Waiting on author to review label Jun 6, 2024
@eth-bot eth-bot changed the title add sentence to make atomicBatch more explicitly one-chain-at-a-time Update EIP-5792: add sentence to make atomicBatch more explicitly one-chain-at-a-time Jun 6, 2024
@bumblefudge bumblefudge changed the title Update EIP-5792: add sentence to make atomicBatch more explicitly one-chain-at-a-time Update EIP-5792: make atomicBatch chain-specific per-batch, not per-call (both in protocol messages and in normative prose) Jun 6, 2024
Copy link
Contributor

@lukasrosario lukasrosario left a comment

Choose a reason for hiding this comment

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

requesting that we make the expected behavior for calls across chains clearer. specifically, do calls on one chain need to land before executing calls on the next chain? or can they be made closer to in parallel?

@bumblefudge
Copy link
Contributor Author

i would assume parallel but what do i know about defi. my intuition is that either way, it should be a SHOULD for now and future EIPs would extend the capability definition (or propose an alternative) to be more explicit or opinionated unless people working on such use cases or mechanisms want to speak up and argue one way or the other?

@pedrouid
Copy link
Contributor

Hey @lukasrosario that's a great question but I think it applies for both single-chain and multi-chain Atomic Batching

What is the intended behavior for ordering even in a single-chain scenario? Should the Wallet respect the order of the items in the array? Ordering can be lost depending on the serialization and deserialization process involved

I think the Atomic Batching needs to have clearer expectations even without the changes included in this PR

@drortirosh
Copy link
Contributor

drortirosh commented Jun 20, 2024

i'm not sure how can you provide such behavior: there is no way to guarantee cross-chain atomic batch, so it is "Best effort" at best.

It is an excellent feature to have - but not something we can provide at the wallet level API.
it has a lot of security concerns around it - see all the discussions around "intents"

@pedrouid
Copy link
Contributor

pedrouid commented Aug 8, 2024

This can be closed in favor of the already merged #8771

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-review Waiting on author to review c-update Modifies an existing proposal t-interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants