Foundational Store Substreams integration#661
Merged
Conversation
UlysseCorbeil
force-pushed
the
feature/establish-foundationnal-store-intrinsics
branch
from
3550837 to
b1f02fb
Compare
Contributor
Author
|
Sorry about the commit history, I had to rebase so github didnt show hundreds of "changed" files coming from a merge |
maoueh
requested changes
Aug 26, 2025
Comment on lines
458
to
464
| if s, ok := r.(string); ok { | ||
| e := fmt.Errorf("wasmtime error: %s", s) | ||
| p.execoutStorage.ConfigMap[executorName].WriteDeterministicError(ctx, execOutput.Clock().Number, e) | ||
| out.err = e | ||
| return | ||
| } | ||
| panic(fmt.Errorf("unknown error: %s", r)) |
Contributor
There was a problem hiding this comment.
Would like to understand better this change. Why it would be wasmtime here actually? We have more than one vm processor.
Also, when recovering from panic, just checking for r.(string) is usually not enough, you need to check for error too.
Contributor
Author
There was a problem hiding this comment.
There was a silent panic here, it just failed without crashing iirc so I added an error we can see and if it ever reaches that panic, means something went very wrong
Comment on lines
430
to
452
| // Deserialize | ||
| var req pbstore.GetAllRequest | ||
| if err := proto.Unmarshal(reqData, &req); err != nil { | ||
| call.ReturnError(fmt.Errorf("failed to unmarshal GetAllRequest: %w", err)) | ||
| stack[0] = 0 | ||
| return | ||
| } | ||
|
|
||
| resp, err := call.DoFoundationalStoreGetAll(storeIndex, req.BlockNumber, req.Keys) | ||
| if err != nil { | ||
| call.ReturnError(fmt.Errorf("foundational store error: %w", err)) | ||
| stack[0] = 0 | ||
| return | ||
| } | ||
|
|
||
| // Serialize response | ||
| respData, err := proto.Marshal(resp) | ||
| if err != nil { | ||
| call.ReturnError(fmt.Errorf("failed to marshal GetAllResponse: %w", err)) | ||
| stack[0] = 0 | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Maybe we should create an helper for those as we will also need in JavaScript!
billettc
deleted the
feature/establish-foundationnal-store-intrinsics
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.