drop query proofs in casting and agoric-cli#11022
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
72715c5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c538651d.agoric-sdk.pages.dev |
| Branch Preview URL: | https://ta-drop-query-proofs.agoric-sdk.pages.dev |
michaelfig
left a comment
There was a problem hiding this comment.
Please leave signposts so the user is not confused.
| }, | ||
| 'optimistic', | ||
| ) | ||
| .option( |
There was a problem hiding this comment.
Please don't make an unnecessary breaking change here. --proof=none is still legitimate.
| .option( | |
| .option( | |
| '--proof <none>', | |
| `set proof mode (currently only 'none' is supported)`, | |
| value => { | |
| assert.equal( | |
| value, | |
| 'none', | |
| X`--proof can only be 'none'`, | |
| TypeError, | |
| ); | |
| return value; | |
| }, | |
| 'none', | |
| ) | |
| .option( |
There was a problem hiding this comment.
fair. I'll include something about the option being deprecated since it doesn't do anything.
There was a problem hiding this comment.
I edited the above to suggest something better than before. Please take it into account.
| console.error('getProvenDataAtHeight', height, 'is no longer supported'); | ||
| throw makeError(X`Verified queries are no longer supported`); |
There was a problem hiding this comment.
| console.error('getProvenDataAtHeight', height, 'is no longer supported'); | |
| throw makeError(X`Verified queries are no longer supported`); | |
| console.error('getProvenDataAtHeight', height, 'is no longer supported; use', { proof: 'none' }); | |
| throw makeError(X`Verified queries are no longer supported; use { proof: 'none' }`); |
There was a problem hiding this comment.
PR Overview
This pull request removes support for query proofs as the underlying JS implementation has been dropped, and updates the related APIs and CLI options accordingly.
- In follower-cosmjs.js, the default proof mode is changed from "optimistic" to "none" and the deprecated function getProvenDataAtHeight now logs an error and throws an exception.
- In agoric-cli, the --proof option is removed from the "follow" command so that users are no longer shown this unsupported option.
Reviewed Changes
| File | Description |
|---|---|
| packages/casting/src/follower-cosmjs.js | Update default proof mode and deprecate getProvenDataAtHeight. |
| packages/agoric-cli/src/main.js | Remove the CLI option related to query proofs. |
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
7540e4e to
26ea874
Compare
26ea874 to
9506463
Compare
9506463 to
72715c5
Compare
requested changes were applied verbatim
|
This pull request has been removed from the queue for the following reason: The merge conditions cannot be satisfied due to failing checks: You may have to fix your CI before adding the pull request to the queue again. If you want to requeue this pull request, you can post a |
|
The only failure is in test-multichain-e2e which is flaking. Bypassing now. |
incidental
Description
CosmJS dropped support for query proofs because the JS implementation was removed from ics23.
There is no supported implementation so this removes the feature.
It leaves the code structure intact in case we can find another implementation, so the earlier API can be restored.
It drops the option entirely from agoric-cli so users aren't shown it in help and autocomplete.
Security Considerations
Loss of client-side proofs.
Scaling Considerations
none
Documentation Considerations
Code comments
Testing Considerations
CI
Upgrade Considerations
I can't find anything that was using it.