feat(cosmos): Add a "CapData" vstorage RPC endpoint #8056

gibson042 · 2023-07-17T17:00:19Z

Description

RPC requests with path "/agoric.vstorage.Query/CapData" will get back vstorage data that has been interpreted as CapData and (lossily) formatted in the style of agoric follow or scripts/get-flattened-publication.sh.

Security Considerations

There is no expected change in security posture. Clients can continue to request the raw data; this interface just provides a convenient way for them to keep up with potential future changes in CapData representation without having to run local transformations.

Scaling Considerations

Use of the new endpoint represents more work for an RPC node and generation of some more Go garbage, but the impact should be negligible (and the responses theirselves will be smaller).

Documentation Considerations

We don't seem to have any documentation of RPC endpoints. Perhaps @Tyrosine22 has thoughts on how to address that?

Testing Considerations

I'm still considering options for how to test the CapData decoder at golang/cosmos/x/vstorage/capdata/capdata.go , and will probably add some static tests mirroring those in https://github.com/endojs/endo/tree/master/packages/marshal but would also like to roll something more into endojs/endo#1588 where endo publishes a generic conformance testing artifact that is imported and used in the testing of this Go package.

Fixes #7581

chore(cosmos): Fix Go formatting

turadg

I don't know Go well enough to review the implementation. The thrust of the design makes sense to me. I'd like to see some tests that demonstrate the interface for clients. (e.g. example responses)

dckc

examples / tests are critical, I think

now I see the relevant tests

dckc · 2023-07-19T18:14:56Z

now I see the relevant tests
oops... that was a different PR. (I won't renew my request for changes just now, though)

turadg · 2023-07-24T15:21:07Z

golang/cosmos/x/vstorage/keeper/grpc_query.go

+var capDataRemotableValueFormats = map[string]string{
+	FormatRemotableAsObject: FormatRemotableAsObject,
+	FormatRemotableAsString: FormatRemotableAsString,
+	// No default.


with no default, is it an error when the request omits this parameter? object seems to me the appropriate default as it's the most generic.

Yes, I decided that omitting FormatRemotableAsString should be an error because all valid values are lossy w.r.t. CapData.

ah I see. that rationale would be worth including in the comment

turadg · 2023-07-24T15:22:28Z

golang/cosmos/proto/agoric/vstorage/query.proto

+  // * "object" represents each Remotable as an `{ id, allegedName }` object.
+  // * "string" represents each Remotable as a bracketed string such as `[Alleged: IST brand {}]`.


this is important documentation. is it published somewhere using godoc ?

Not that I know of (as mentioned in the PR summary, we don't seem to have any documentation of RPC endpoints). Does anyone have a place where they would expect to find it?

we don't seem to have any documentation of RPC endpoints

It's far from perfect, but we have >0 for inter protocol RPC stuff: https://github.com/Agoric/agoric-sdk/tree/master/packages/inter-protocol#reading-data-off-chain

As to the rest, yes, it's been a hole in our docs for some time; I put what notes I learn in...

Cosmos / tendermint RPC doc links · Issue #657 · Agoric/documentation

In some sense, this is a different issue, since that issue was about documentation of the form "Agoric stuff is based on cosmos-sdk; go read XYZ to find out how the lower level works." It turns out that there's no obvious candidate for XYZ, btw.

Does anyone have a place where they would expect to find it?

The inter-protocol precedent suggests a readme under golang/cosmos/x/vstorage

Added golang/cosmos/x/vstorage/README.md.

Also, it turns out that the documentation is available via godoc of the generated pb.go.

dckc

I don't see how clients can work if Id is left out of remotable string rendering.

dckc · 2023-07-24T16:43:20Z

golang/cosmos/x/vstorage/keeper/grpc_query.go

+// capdataRemotableToString represents a Remotable as a bracketed string
+// containing its alleged name (e.g., "[Foo {}]").
+func capdataRemotableToString(r *capdata.CapdataRemotable) interface{} {
+	iface := "Remotable"
+	if r.Iface != nil || *r.Iface != "" {
+		iface = *r.Iface
+	}
+	return fmt.Sprintf("[%s {}]", iface)
+}


In order to prevent clients from relying on the Iface (aka debug name) for correctness, the rendering of a remotable must preserve the Id. Including the Iface at all is risky.

Per #7581, this attempts to align with agoric follow -o jsonlines:

agoric --sdk follow :published.vaultFactory.manager0.metrics -B https://devnet.agoric.net/network-config --proof none -o jsonlines | jq . { "liquidatingCollateral": { "brand": "[Alleged: SEVERED: IbcATOM brand {}]", "value": "0" }, …

Who constitutes the right group to discuss well-motivated deviation? @arirubinstein?

dckc · 2023-07-24T16:45:43Z

golang/cosmos/x/vstorage/keeper/grpc_query.go

+	if r.Iface != nil || *r.Iface != "" {
+		iface = *r.Iface
+		iface, _ = strings.CutPrefix(iface, "Alleged: ")
+		iface, _ = strings.CutSuffix(iface, " brand")


Why cut off "brand"? Why should this code have knowledge of ERTP?

cc @erights

This attempts to align with scripts/get-flattened-publication.sh. I'm open to deviation here as well, but again unclear about authority for making such a decision.

gibson042 · 2023-07-24T17:25:49Z

This PR now features what AFAICT is the most comprehensive Go testing in the repository, for both the capdata decoder and the CapData gRPC endpoint.

erights

Title says "CapData". Did you mean "smallcaps"?

… JSON encoding

…Data remotable representations

gibson042 · 2023-07-27T16:56:35Z

Title says "CapData". Did you mean "smallcaps"?

Nope, it's formatting CapData regardless of smallcaps vs. legacy encoding.

gibson042 · 2023-07-27T17:06:26Z

We decided at the cosmic-swingset meeting that reducing lossiness is more important than preserving precise compatibility with agoric follow and get-flattened-publication.sh formats, both of which were based on somewhat ad hoc decisions. As of the latest push, the new Remotable representations are

object: { "id": $slotId, "allegedName": "$unprefixedName" }, e.g. { …, "brand": { "id": "board007", "allegedName": "IST brand" } }
string: "[$unmodifiedName <$slotId>]", e.g. { …, "brand": "[Alleged: IST brand <board007>]" }

@turadg, please confirm that this works for you.

dckc

I like it

turadg

WFM. Glad to see the eventual design had support in the full cosmic-swingset group.

feat(cosmos): Add a "CapData" vstorage RPC endpoint

gibson042 added 3 commits July 17, 2023 12:39

feat(cosmos): Add a vstorage package for decoding CapData

8bdd7cb

feat(cosmos): Add a "CapData" vstorage RPC endpoint

8943f2f

Fixes #7581

chore(cosmos): Regenerate protobuf code

b68a5ba

gibson042 requested review from turadg, dckc and michaelfig July 17, 2023 17:00

fixup! feat(cosmos): Add a "CapData" vstorage RPC endpoint

22c7d49

chore(cosmos): Fix Go formatting

turadg reviewed Jul 17, 2023

View reviewed changes

dckc previously requested changes Jul 17, 2023

View reviewed changes

chore(cosmos): Improve vstorage capdata error messages

9356509

gibson042 force-pushed the gibson-7581-rpc-capdata-formatting branch from 539e2a3 to d8522cc Compare July 21, 2023 19:55

test(cosmos): Test vstorage capdata

36b921d

gibson042 force-pushed the gibson-7581-rpc-capdata-formatting branch from d8522cc to 36b921d Compare July 21, 2023 20:12

turadg reviewed Jul 24, 2023

View reviewed changes

dckc requested changes Jul 24, 2023

View reviewed changes

test(cosmos): Test gRPC endpoint "/agoric.vstorage.Query/CapData"

a7f17fe

gibson042 force-pushed the gibson-7581-rpc-capdata-formatting branch from b39ca47 to a7f17fe Compare July 24, 2023 17:00

gibson042 added 2 commits July 25, 2023 23:28

chore(cosmos): Improve vstorage/keeper/grpc_query.go doc comments

e4bfad3

chore(cosmos): Add vstorage README

dff7e93

erights reviewed Jul 26, 2023

View reviewed changes

gibson042 added 2 commits July 27, 2023 12:50

chore(cosmos): Avoid special treatment of HTML characters in vstorage…

b4fdcc9

… JSON encoding

feat(cosmos): Always include alleged name and slot id in vstorage Cap…

e2cbffa

…Data remotable representations

gibson042 requested a review from turadg July 27, 2023 16:56

dckc approved these changes Jul 28, 2023

View reviewed changes

turadg approved these changes Jul 28, 2023

View reviewed changes

gibson042 added this pull request to the merge queue Jul 28, 2023

Merged via the queue into master with commit 91f5e93 Jul 28, 2023
59 checks passed

gibson042 deleted the gibson-7581-rpc-capdata-formatting branch July 28, 2023 22:10

gibson042 mentioned this pull request Jul 31, 2023

chore(cosmos): Update doc comments for accuracy #8119

Merged

mhofman pushed a commit that referenced this pull request Aug 7, 2023

Merge pull request #8056 from Agoric/gibson-7581-rpc-capdata-formatting

360924c

feat(cosmos): Add a "CapData" vstorage RPC endpoint

mhofman mentioned this pull request Feb 18, 2024

Cherry-pick upgrade-14 cosmos changes #8869

Merged

mhofman pushed a commit that referenced this pull request Feb 18, 2024

Merge pull request #8056 from Agoric/gibson-7581-rpc-capdata-formatting

8cda00a

feat(cosmos): Add a "CapData" vstorage RPC endpoint

JimLarson mentioned this pull request Mar 27, 2024

chore: remove obsolete get-flattened-publication.sh script #9159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cosmos): Add a "CapData" vstorage RPC endpoint #8056

feat(cosmos): Add a "CapData" vstorage RPC endpoint #8056

gibson042 commented Jul 17, 2023

turadg left a comment

dckc left a comment

dckc commented Jul 19, 2023

turadg Jul 24, 2023

gibson042 Jul 24, 2023

turadg Jul 24, 2023

gibson042 Jul 26, 2023

turadg Jul 24, 2023

gibson042 Jul 24, 2023

dckc Jul 24, 2023

gibson042 Jul 26, 2023 •

edited

Loading

gibson042 Jul 26, 2023

dckc left a comment

dckc Jul 24, 2023

gibson042 Jul 24, 2023 •

edited

Loading

dckc Jul 24, 2023

gibson042 Jul 24, 2023

gibson042 commented Jul 24, 2023

erights left a comment

gibson042 commented Jul 27, 2023

gibson042 commented Jul 27, 2023

dckc left a comment

turadg left a comment

		// * "object" represents each Remotable as an `{ id, allegedName }` object.
		// * "string" represents each Remotable as a bracketed string such as `[Alleged: IST brand {}]`.

feat(cosmos): Add a "CapData" vstorage RPC endpoint #8056

feat(cosmos): Add a "CapData" vstorage RPC endpoint #8056

Conversation

gibson042 commented Jul 17, 2023

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

turadg left a comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

dckc commented Jul 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 commented Jul 24, 2023

erights left a comment

Choose a reason for hiding this comment

gibson042 commented Jul 27, 2023

gibson042 commented Jul 27, 2023

dckc left a comment

Choose a reason for hiding this comment

turadg left a comment

Choose a reason for hiding this comment

gibson042 Jul 26, 2023 •

edited

Loading

gibson042 Jul 24, 2023 •

edited

Loading