Conversation
|
This current approach dumps the entire body, not the extract value. This is inconsistent with the old APIs. It should only output the value. FYI @piotr-roslaniec , this probably needs to reflect the output of the ManifestV2 multi-extractions as well. The approach I took in my other PR is naive. |
|
Okay lets merge in the V2 and then i will rebase and take a look here. |
0xJepsen
force-pushed
the
feat/return_value
branch
from
f2921d0 to
a6402ec
Compare
notary/src/proxy.rs
Outdated
| let path = manifest.response.body.json_path().clone(); | ||
| let body = serde_json::from_slice(&response.notary_response_body.clone().body.unwrap()).unwrap(); | ||
| let value = get_value_from_json_path(&body, &path); | ||
| let manifest_hash = KeccakHasher::hash(serde_json::to_string(&manifest)?.as_bytes()); |
There was a problem hiding this comment.
@piotr-roslaniec we need to make the hash like this so that the same proof produces the same hash. This is because in the on chain verifier, we shouldn't let users submit duplicate proofs. and we need to enable users to recreate the root hash.
There was a problem hiding this comment.
I see. This is not going to work for HTML extraction. I will look into that.
There was a problem hiding this comment.
How about something like this? https://github.com/pluto/web-prover/compare/feat/return_value...feat/return_value_2?expand=1
There was a problem hiding this comment.
yes these seems reasonable as long as the keccak digest is deterministic such that the users can recreate it if need be.
There was a problem hiding this comment.
I like this simple approach from Piotr, let's update to use that and merge this PR.
|
Would also be nice to have support for these extractions all the way out to the contract. I created an issue on solidity-verifier for that: pluto/solidity-verifier#4 (comment) |
|
This should be ready for review |
|
|
||
| validate_notarization_legal(manifest, request, response)?; | ||
| let path = manifest.response.body.json_path().clone(); | ||
| let body = serde_json::from_slice(&response.notary_response_body.clone().body.unwrap()).unwrap(); |
There was a problem hiding this comment.
This is still breaking for the HTML format. Shall we disable HTML support for the time, or reconsider with this approach: #563 (comment) ?
3 participants
Pull Request
This is a very small change but very important feature for the user. This change just ensures that the data from the response (that we are proving) is returned to the user.