Skip to content

Add field in callbacks data for custom calldata #8353

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
Merged
merged 11 commits into from
May 28, 2025
Merged

Add field in callbacks data for custom calldata #8353

merged 11 commits into from
May 28, 2025

Conversation

@AdityaSripal
Copy link
Contributor

@AdityaSripal AdityaSripal commented May 7, 2025

Description

closes: #XXXX

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

@AdityaSripal AdityaSripal mentioned this pull request May 9, 2025
Closed
@codecov-commenter
Copy link

codecov-commenter commented May 14, 2025 •
edited by codecov bot
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.68%. Comparing base (a5ad744) to head (52b6a36).
Report is 11 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8353      +/-   ##
==========================================
+ Coverage   61.62%   61.68%   +0.05%     
==========================================
  Files         299      299              
  Lines       21462    21494      +32     
==========================================
+ Hits        13227    13259      +32     
  Misses       7643     7643              
  Partials      592      592
Flag Coverage Δ
08-wasm 66.28% <ø> (ø)
e2e 1.18% <ø> (ø)
ibc-go 67.77% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AdityaSripal AdityaSripal marked this pull request as ready for review May 21, 2025 15:45
gjermundgaraba
gjermundgaraba reviewed May 21, 2025
View reviewed changes
modules/apps/callbacks/types/callbacks.go Outdated

calldata, err := hex.DecodeString(calldataStr)
if err != nil {
return nil
Copy link
Contributor

@gjermundgaraba gjermundgaraba May 21, 2025

Choose a reason for hiding this comment

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

Are we sure this should not return an error?

@AdityaSripal
Copy link
Contributor Author

AdityaSripal commented May 22, 2025

Introduced tighter validation of callback data in memo. Though adding errors in all the helper functions is a bit ugly.

Let me know if you would prefer panics.

I'm finishing up testing this when I am back

@gjermundgaraba
Copy link
Contributor

gjermundgaraba commented May 22, 2025

Sounds good. Errors seems fine, easier to test 🤷

DeshErBojhaa
DeshErBojhaa reviewed May 28, 2025
View reviewed changes
Copy link
Contributor

@DeshErBojhaa DeshErBojhaa left a comment

Choose a reason for hiding this comment

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

One minor suggestion. Other than that, LGTM!

AdityaSripal
AdityaSripal commented May 28, 2025
View reviewed changes
modules/apps/callbacks/types/callbacks_test.go
nil,
},
{
"success: source callback with calldata",
Copy link
Contributor Author

@AdityaSripal AdityaSripal May 28, 2025

Choose a reason for hiding this comment

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

Note: we allow calldata on either side in ibc-go. EVM contract keeper will enforce calldata only set for dest calldata

gjermundgaraba
gjermundgaraba reviewed May 28, 2025
View reviewed changes
gjermundgaraba
gjermundgaraba reviewed May 28, 2025
View reviewed changes
Copy link
Contributor

@gjermundgaraba gjermundgaraba left a comment

Choose a reason for hiding this comment

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

It looks good to me, but I would like to understand why codecov thinks the lines are not covered 🤔

DeshErBojhaa
DeshErBojhaa approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@DeshErBojhaa DeshErBojhaa left a comment

Choose a reason for hiding this comment

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

utACK!

gjermundgaraba
gjermundgaraba approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@gjermundgaraba gjermundgaraba left a comment

Choose a reason for hiding this comment

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

Heck yeah! Great work 🙏

@gjermundgaraba gjermundgaraba merged commit 7d579b9 into main May 28, 2025
51 checks passed
@gjermundgaraba gjermundgaraba deleted the aditya/cb-custom-calldata branch May 28, 2025 14:22
DeshErBojhaa pushed a commit that referenced this pull request Jun 3, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
68c56d4 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
mergify bot pushed a commit that referenced this pull request Jun 5, 2025
@AdityaSripal @gjermundgaraba @mergify
feat: add field in callbacks data for custom calldata (#8353)
aaa02c2 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
(cherry picked from commit 7d579b9)
@mergify mergify bot mentioned this pull request Jun 5, 2025
Merged
10 tasks
gjermundgaraba added a commit that referenced this pull request Jun 5, 2025
@mergify @AdityaSripal @gjermundgaraba
feat: add field in callbacks data for custom calldata (#8353) (#8477)
ad65c38 
Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 6, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
2722971 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 6, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
d586a86 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 7, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
31a1dc4 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 9, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
22b3ac8 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 10, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
86efebd 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 12, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
de89e95 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 13, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
ba6786d 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 17, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
ea06284 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
DeshErBojhaa pushed a commit that referenced this pull request Jun 18, 2025
@AdityaSripal @gjermundgaraba @DeshErBojhaa
feat: add field in callbacks data for custom calldata (#8353)
79ffae0 
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gjermundgaraba gjermundgaraba gjermundgaraba approved these changes

@srdtrk srdtrk Awaiting requested review from srdtrk

+1 more reviewer

@DeshErBojhaa DeshErBojhaa DeshErBojhaa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AdityaSripal @codecov-commenter @gjermundgaraba @DeshErBojhaa