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 wallet extract-id CLI #10892

Merged
merged 4 commits into from
Jan 24, 2025
Merged

fix wallet extract-id CLI #10892

merged 4 commits into from
Jan 24, 2025

Conversation

turadg
Copy link
Member

@turadg turadg commented Jan 24, 2025

closes: #10891

Description

A recent refactor of agoric-cli introduced an error. It wasn't caught by typecheck because the callsite was any.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

This PR doesn't have a runtime regression test because this is a very uncommon code path which does have some CI coverage in a3p-integration (1d01880)

Upgrade Considerations

Not run on chain.

@turadg turadg requested review from dckc and mujahidkay January 24, 2025 22:27
@turadg turadg requested a review from a team as a code owner January 24, 2025 22:27
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

looks good

@@ -132,6 +133,7 @@ harden(execSwingsetTransaction);
/**
*
* @param {MinimalNetworkConfig} net
* @returns {ParamsSDKType}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure JSON output from agd always directly corresponds with protobuf stuff. I wouldn't be surprised if it does in this case, but did you happen to verify experimentally? Or maybe there's a caller that clearly relies on 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.

Yeah there's a caller.
Screenshot 2025-01-24 at 2 41 02 PM

@turadg turadg added automerge:rebase Automatically rebase updates, then merge bypass:integration Prevent integration tests from running on PR labels Jan 24, 2025
@mergify mergify bot merged commit b2cb709 into master Jan 24, 2025
89 of 96 checks passed
@mergify mergify bot deleted the 10891-fix-cli branch January 24, 2025 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge bypass:integration Prevent integration tests from running on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wallet extract-id CLI fails
2 participants