-
Notifications
You must be signed in to change notification settings - Fork 590
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: add custom querier to wasm image #7559
feat: add custom querier to wasm image #7559
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think we can worry about the speed issues you brought up in a follow up, or if it becomes a bottleneck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return json.Marshal(aggregatedPublicKeys.Marshal()) | ||
case customQuery.AggregateVerify != nil: | ||
if len(customQuery.AggregateVerify.Message) != MessageSize { | ||
return nil, fmt.Errorf("invalid message length, must be a 32bytes hash: %x", customQuery.AggregateVerify.Message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] I guess it doesn't hurt to add the obtained len to the error message. Also, you could use MessageSize instead of an hardcoded "32" in the message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, added it now :)
Quality Gate passed for 'ibc-go'Issues Measures |
Description
This adds the custom querier needed for the union light client to the WASM simapp image.
This change also makes the usual amd64 image unusable on arm64 machines (Mac M1/2/etc.), so I've updated the build WASM image pipeline to build for amd64 and arm64 at the same time.
Unfortunately, the speed of that build has taken a significant hit due to the virtual nature of QEMU that is needed to build for arm64 on an amd64 machine. It can take up to around 25 minutes (https://github.com/cosmos/ibc-go/actions/runs/11817324167).
It is possible to improve this, but it would require a more complex pipeline that builds the images in a separate job with separate machines with separate tags (using the matrix strategy) and then downloads those images and pushes them together with the same tag. Or something very similar using Docker manifest - I haven't fully dived into the details yet (but it would be more complex for sure and take more time to set up). If this would be a big issue, I am open to doing that or finding other solutions.
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.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.