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.

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

fix: join context with proxy #955

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MatejVukosav
Copy link
Member

Joining context throws

 ERROR calimero_node: Failed handling user command: 
   0: transport error: relayer response (500 Internal Server Error): left error: error while querying contract: handler error: [Function call returned an error: wasm execution failed with error: MethodResolveError(MethodNotFound)]

@MatejVukosav MatejVukosav self-assigned this Nov 8, 2024
@MatejVukosav MatejVukosav changed the title Fix join context with proxy fix: join context with proxy Nov 8, 2024
Comment on lines +357 to +360
let proxy_contract = self
.get_proxy_contract(context_id)
.await
.unwrap_or_else(|_| "".to_owned());
Copy link
Member

Choose a reason for hiding this comment

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

if there's an error, we should handle it

Copy link
Member Author

Choose a reason for hiding this comment

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

Error is that it does not exists.

Copy link
Member

@miraclx miraclx Nov 8, 2024

Choose a reason for hiding this comment

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

if it doesn't exist, then there's a deeper issue, the config contract didn't record the context, and if that's the case, we shouldn't be here at all (context creation should've failed).

Perhaps we need to tweak near tx submission to await cross contract calls, so we can appropriately determine if the whole chain succeeded.

Copy link
Member Author

Choose a reason for hiding this comment

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

It exists on owner side but it does not exists for invited peer

Copy link
Member

Choose a reason for hiding this comment

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

It exists on owner side but it does not exists for invited peer

get_proxy_contract calls the contract, and it would only return that error if the context doesn't exist on the config contract

Copy link
Member

Choose a reason for hiding this comment

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

It exists on owner side but it does not exists for invited peer

interesting that there's an invitation payload to begin with, add_member should've failed as well if the context doesn't exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Invited peer joined context but catchup failed.

Copy link
Member

Choose a reason for hiding this comment

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

what's the error?

Copy link
Member Author

Choose a reason for hiding this comment

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

in description

Copy link
Member

Choose a reason for hiding this comment

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

doesn't explain why ignoring the error is the solution

the propose method wasn't found because it was calling the wrong contract, which this PR fixes

but this line, you said was because the context didn't exist, in which case, we shouldn't be ignoring the error but figuring out why that's the case.

but if the error no longer happens, then we don't need this line, and any errors that occur here should be handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants