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.

Sign up for GitHub

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

Jump to bottom

feat: blockchain agnostic inputs #89

Merged
merged 1 commit into from
Jan 25, 2024
Merged

feat: blockchain agnostic inputs #89

merged 1 commit into from
Jan 25, 2024

Conversation

ZzzzHui
Copy link
Contributor

@ZzzzHui ZzzzHui commented Sep 25, 2023

Not sure if this draft is somewhat close to the expected solution? Please let me know if it's not :)

@ZzzzHui ZzzzHui requested a review from guidanoli September 25, 2023 17:59
@ZzzzHui ZzzzHui self-assigned this Sep 25, 2023
@ZzzzHui ZzzzHui linked an issue Sep 25, 2023 that may be closed by this pull request
Blockchain-agnostic Inputs #84
Closed
@guidanoli
Copy link
Collaborator

guidanoli commented Sep 25, 2023 •
edited
Loading

Almost there. The algorithm is actually this:

  1. Encode the blob using abi.encodeWithSignature.
  2. Check if the size of the blob exceeds the limit.
  3. Return the hash of the blob.

@ZzzzHui
Copy link
Contributor Author

ZzzzHui commented Sep 26, 2023

Almost there. The algorithm is actually this:

  1. Encode the blob using abi.encodeWithSignature.
  2. Check if the size of the blob exceeds the limit.
  3. Return the hash of the blob.

These were implemented in the LibInput, right

@guidanoli
Copy link
Collaborator

These were implemented in the LibInput, right

These should be implemented in LibInput.

@ZzzzHui
Copy link
Contributor Author

ZzzzHui commented Sep 27, 2023

I meant they were already implemented, right?

@guidanoli
Copy link
Collaborator

guidanoli commented Sep 27, 2023 •
edited
Loading

Not quite. We won't be hashing the metadata and payload separately. Instead, we'll ABI-encode the metadata and payload, and then hash the whole thing.

@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch 2 times, most recently from 059582b to a682cd3 Compare September 28, 2023 16:50
@ZzzzHui ZzzzHui marked this pull request as ready for review September 28, 2023 16:53
guidanoli
guidanoli reviewed Sep 28, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@ZzzzHui ZzzzHui linked an issue Oct 9, 2023 that may be closed by this pull request
Rename fields in InputAdded event. #2
Closed
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from a682cd3 to e45e4a2 Compare October 9, 2023 15:25
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from e45e4a2 to 33398e5 Compare October 11, 2023 18:10
@guidanoli guidanoli removed the request for review from xdaniortega October 24, 2023 15:07
@guidanoli guidanoli changed the base branch from main to next October 27, 2023 18:46
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from 33398e5 to 0002d32 Compare October 31, 2023 16:49
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: 14e3755

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

guidanoli
guidanoli reviewed Oct 31, 2023
View reviewed changes
onchain/.changeset/mean-news-float.md Outdated Show resolved Hide resolved
onchain/.changeset/mean-news-float.md Outdated Show resolved Hide resolved
onchain/.changeset/mean-news-float.md Outdated Show resolved Hide resolved
guidanoli
guidanoli reviewed Oct 31, 2023
View reviewed changes
onchain/rollups/contracts/inputs/InputBox.sol Outdated Show resolved Hide resolved
onchain/rollups/contracts/inputs/InputBox.sol Outdated Show resolved Hide resolved
onchain/rollups/contracts/library/LibInput.sol Outdated Show resolved Hide resolved
onchain/rollups/contracts/library/LibInput.sol Outdated Show resolved Hide resolved
onchain/rollups/test/foundry/inputs/InputBox.t.sol Outdated Show resolved Hide resolved
@guidanoli
Copy link
Collaborator

guidanoli commented Oct 31, 2023 •
edited
Loading

We can make this PR have two commits.

@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from 6212430 to 14e3755 Compare November 2, 2023 11:13
@ZzzzHui ZzzzHui requested a review from guidanoli November 2, 2023 11:20
This was unlinked from issues Nov 21, 2023
Include the DApp address in the EvmAdvance input #144
Closed
Include the chain ID in the EvmAdvance input #145
Closed
@guidanoli
Copy link
Collaborator

Ok, #136 has been merged.

@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from e8523ea to 4471b6e Compare November 21, 2023 18:22
@ZzzzHui ZzzzHui changed the base branch from next to main November 21, 2023 18:25
@ZzzzHui ZzzzHui dismissed pedroargento’s stale review November 21, 2023 18:25

The base branch was changed.

@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from 4471b6e to cb0e002 Compare November 22, 2023 16:42
@ZzzzHui ZzzzHui changed the base branch from main to next November 23, 2023 14:10
@guidanoli guidanoli removed a link to an issue Jan 17, 2024
Rename fields in InputAdded event. #2
Closed
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from cb0e002 to eabc674 Compare January 23, 2024 13:21
@ZzzzHui ZzzzHui changed the base branch from next to main January 23, 2024 13:21
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from eabc674 to 77e0df4 Compare January 23, 2024 16:37
@ZzzzHui ZzzzHui mentioned this pull request Jan 23, 2024
Closed
@ZzzzHui ZzzzHui linked an issue Jan 24, 2024 that may be closed by this pull request
Adjust nomenclature used in IInputBox #203
Closed
guidanoli
guidanoli previously approved these changes Jan 24, 2024
View reviewed changes
@guidanoli guidanoli dismissed their stale review January 24, 2024 14:58

Waiting on changeset file update.

@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from 77e0df4 to ae8dd05 Compare January 24, 2024 15:25
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from ae8dd05 to d0e8bc1 Compare January 24, 2024 18:36
@ZzzzHui ZzzzHui force-pushed the feature/blockchain-agnostic-inputs branch from d0e8bc1 to 0f88032 Compare January 25, 2024 09:03
@ZzzzHui ZzzzHui merged commit 13eb18a into main Jan 25, 2024
3 checks passed
@ZzzzHui ZzzzHui deleted the feature/blockchain-agnostic-inputs branch January 25, 2024 15:37
@ZzzzHui ZzzzHui mentioned this pull request Jan 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gligneul gligneul gligneul left review comments

@mpolitzer mpolitzer mpolitzer left review comments

@guidanoli guidanoli guidanoli approved these changes

@pedroargento pedroargento pedroargento left review comments

Assignees

@ZzzzHui ZzzzHui

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@ZzzzHui @guidanoli @mpolitzer @pedroargento @gligneul