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

Adjust nomenclature used in IInputBox #203

Closed
guidanoli opened this issue Jan 17, 2024 · 6 comments · Fixed by #89
Closed

Adjust nomenclature used in IInputBox #203

guidanoli opened this issue Jan 17, 2024 · 6 comments · Fixed by #89
Assignees
@ZzzzHui
Labels
A-contracts Area: contracts T-debt Type: debt
Milestone
2.0.0

Comments

@guidanoli
Copy link
Collaborator

📚 Context

On #201, we've made some changes to address #2. Most importantly...

  • Renamed input parameter as payload
  • Renamed MAX_INPUT_SIZE constant as MAX_INPUT_PAYLOAD_SIZE
  • Added PayloadTooLarge custom error

With #84, however, the Server Manager will not write just the payload to the rx buffer, but the whole input (payload and metadata) encoded as a Solidity function call.

✔️ Solution

We need to make the following adjustments to the IInputBox interface...

  • Rename MAX_INPUT_PAYLOAD_SIZE constant as MAX_INPUT_SIZE
  • Check if the whole input is smaller than the maximum size, not just the payload
  • Rename PayloadTooLarge custom error as InputTooLarge (and its arguments as well)
@guidanoli guidanoli added T-debt Type: debt D-average A-contracts Area: contracts labels Jan 17, 2024
@guidanoli guidanoli added this to the 3.0.0 milestone Jan 17, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Rollups Unit Jan 17, 2024
@guidanoli guidanoli changed the title Adjust nomenclature used in IInputBox for blockchain-agnostic input Adjust nomenclature used in IInputBox Jan 17, 2024
@guidanoli guidanoli modified the milestones: 3.0.0, 2.0.0 Jan 21, 2024
@ZzzzHui
Copy link
Contributor

ZzzzHui commented Jan 23, 2024

Hey Gui. This issue has overlap with the issue #203 and has already been addressed in PR #89, we can either close this issue or link the PR to this too :)

@guidanoli
Copy link
Collaborator Author

This issue has overlap with the issue #203

What do you mean? This is issue #203. :-)

@guidanoli
Copy link
Collaborator Author

guidanoli commented Jan 23, 2024 •
edited
Loading

we can either close this issue or link the PR to this too :)

Let's link the PR to this issue, then.

@ZzzzHui
Copy link
Contributor

ZzzzHui commented Jan 24, 2024

This issue has overlap with the issue #203

What do you mean? This is issue #203. :-)

Sorry for the typo. I meant #84 😂

@ZzzzHui ZzzzHui self-assigned this Jan 24, 2024
@ZzzzHui ZzzzHui linked a pull request Jan 24, 2024 that will close this issue
feat: blockchain agnostic inputs #89
Merged
@guidanoli
Copy link
Collaborator Author

Sorry for the typo. I meant #84 😂

No problem. What is the overlap, though?

@ZzzzHui
Copy link
Contributor

ZzzzHui commented Jan 24, 2024

The "blockchain agnostic input" issue involves correctly naming input and payload, which covers this issue.

@guidanoli guidanoli moved this from 📋 Backlog to 🚧 In progress in Rollups Unit Jan 25, 2024
@guidanoli guidanoli moved this from 🚧 In progress to 🚀 Done in Rollups Unit Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ZzzzHui ZzzzHui

Labels
A-contracts Area: contracts T-debt Type: debt
Projects
Rollups Unit
Status: 🚀 Done
Milestone
2.0.0
Development

Successfully merging a pull request may close this issue.

feat: blockchain agnostic inputs cartesi/rollups-contracts
2 participants
@guidanoli @ZzzzHui