view_state: add include_proof argument to view state request

[mina86]

Add include_proof to QueryRequest::ViewState requests and view_state
runtime function. If set to true the proof of the response will be
populated with nodes which can be used to verify the
values. Otherwise, the proof won’t be included. For JSON RPC, the
default is for the parameter to be set to false.

Furthermore, deprecate empty proof fields in the query response.
Those were historically always sent and it’s possible that clients
exist which expects them to exist even though they were always empty
in the past. Plan for the future is to include the proof field only
when proof was requested.

Issue: #2076

[mina86]

cc: @blasrodri

[mina86]

@posvyatokum, FYI, we’ll need this PR in the next release.

[matklad]

Codewise, LGTM!

Semantic wise:

• I didn't follow the design discussion here, but I assume we are OK with the way we structure the proofs
• From the changelog, I don't see any mentioning on how to verify the new proofs. I think it's relatively important to include docs into the release.

[matklad]

Could we link some docs explaining how to verify the proof?

[mina86]

Those docs currently don’t exist. It is on my TODO to add them. It’s possible that this might need to go into nomicon.

[matklad]

Process-wise, I would slightly worrying about merging this without docs: docs seem like a phase where we can easily discover some design issues or what not.

I don't think this is blocking, but I would prefer if we did docs before "stabilizing" things.

[nagisa]

Just to verify my own understanding, populating the proof field isn’t a change made specifically in this PR, correct? state_viewer/mod.rs seems to have had all of the relevant code in it already, and the relevant change is the inclusion of the argument.

[mina86]

Correct. We’ve added proof recently in #7593. Priori to that commit (that is, up to and including current testnet release) we’ve always sent empty proof. With #7593 we’re always populating proof. In this PR I want to introduce include_proof filed in the request so that users who don’t care about proofs don’t induce unnecessary load on RPC node.

[nagisa]

So, this defaults to a false if not provided, which is fine, but is also somewhat sketchy given the changelog says:

• The proof field directly within view_state response is currently always
  sent even if proof has not been requested. In the future the field will be
  skipped in those cases. Clients should accept responses with this field
  missing (unless they set include_proof).

Now, I was going to say it is unfortunate that we’re setting ourselves up for an inevitable breaking change, but from what I can tell the reason this disclaimer is there is because something about

    let mut iter = state_update.trie().iter()?;
    iter.remember_visited_nodes(false);

results in iter remembering the visited nodes anyway. Is my understanding correct here? The integration tests would suggest that include_proof = false is actually working the way intuition would suggest it should, though?

---

I would say that it would be safer to change this default to true for now and then add a FIXME on top of this explaining what needs to change in the future. But happy to be convinced otherwise.

[mina86]

remember_visited_nodes works correctly. If include_proof is false than proof field will be an empty vector. The reason for the disclaimer is that I want to remove the field in the future but want to keep it now for compatibility. Note that the proof fields were defined and sent long before we had proof functionality so I want to avoid situation where a client expects the field to be present (even though it historically has always been an empty array) and suddenly breaks when we stop sending the field if its empty.

[folex]

Hi! Do I understand correctly, that include_proof makes view_state to return proof for... what? For the whole response?

% curl -X POST \
     -H 'Content-Type: application/json' \
     -d '{
  "jsonrpc": "2.0",
  "id": "33",
  "method": "query",
  "params": {
    "request_type": "view_state",
    "block_id": 83239132,
    "account_id": "aurora.near",
    "prefix_base64": "cnYAAAAAAAAAAA==",
    "include_proof": true
  }
}' \
     https://archival-rpc.mainnet.near.org

{
    "jsonrpc": "2.0",
    "result": {
        "block_hash": "GooyTyYrCmfGQDu5CkEqTrrqQsrq5NCYhhmgyBAwVjg",
        "block_height": 83239132,
        "proof": [[3,1,0,0,0,...], ...],
        "values": [
            {
                "key": "cnYAAAAAAAAAAA==",
                "proof": [],
                "value": "FwAAAGJpc29udHJhaWxzLnBvb2x2MS5uZWFyAQAAAAURAAAAZGVwb3NpdF9hbmRfc3Rha2UCAAAAe32Enuq0q8TIePjQAAAAAAAAANCY1K9xAAAhAAAAAO22mGQjajGJOySFQx2tMUfkpVXUDNXPTTKzyRBT+q1rqkEE+bWDxRY="
            }
        ]
    },
    "id": "33"
}

Here, there is a result.proof that contains some bytes. But result.values[0].proof is always empty.

Given that, what is result.proof a proof for? For all filtered state? Or just for all contract state?

Do you plan to implement proof for specific values? ie values[N].proof

Thanks in advance!

[mina86]

proof holds all the trie nodes that were accessed in generating the result. values[N].proof is never populated, deprecated and will be removed in the future. test_view_state in integration-tests/src/tests/runtime/state_viewer.rs demonstrates how to verify the proof.