Add specification for Portal JSON-RPC

[KolbyML]

I am still working on this just getting it out there

[pipermerriam]

something like <network-identifier> or <identifier> or <namespace> might be more visually clear.

[kdeme]

Great for getting some of json-rpc requests written down with more detail.

I do think it would be nicer if we can get this all on one place/page, i.e. within the json-rpc specifications that get generated. But I do understand that this could be more work.

As example, the beacon api has very rich explanations on all of their calls: https://ethereum.github.io/beacon-APIs/

[kdeme]

This is rather an attempt to add to the routing table. It might fail according to the rules of the routing table implementation/configuration (e.g. bucket already full, ip limit reached, etc.).

[KolbyML]

I will make this more clear

[kdeme]

At Fluffy we were thinking of having validation + storage on this call but not on the portal_*Offer considering that one seems lower level. But we are not sure about it ourselves, so works either way for us.

[KolbyML]

At Fluffy we were thinking of having validation

If someone gossips invalid data the node gossiped to should temporarily ban the gossiper.

I don't think you can validate everything without making network requests

[bhartnett]

In Fluffy we do local validations without doing network requests (currently just the state network). Sure, the node receiving the offer will validate and reject the data if it is bad but if you can prevent the bad data before sending then why not.

[kdeme]

I'm not sure, but perhaps you could still make it validate the type encoding?

I can see however that for this call you would want to avoid additional network requests.

[KolbyML]

I'm not sure, but perhaps you could still make it validate the type encoding?

Of course the client should validate correct type encodings I will make this more clear

I can see however that for this call you would want to avoid additional network requests.

This call should make 0 network requests

[bhartnett]

For the state network, the only part of the validation that needs a network request is looking up the block header to get the state root. If we skip the state root validation we can still perform all other validations such as decoding, checking the nodeHash matches node in proof, checking structure of the proofs etc and we now do this in Fluffy.

I agree that we shouldn't make network requests but I would suggest applying as many validations as possible to reduce the risk of storing bad data in the db, anything that can be done without a network request.

[KolbyML]

I do think it would be nicer if we can get this all on one place/page, i.e. within the json-rpc specifications that get generated. But I do understand that this could be more work.

As example, the beacon api has very rich explanations on all of their calls: https://ethereum.github.io/beacon-APIs/

I don't think that is as detailed for implementations as the specification for the engine_api is. https://ethereum.github.io/beacon-APIs/ is more user focused, where this documentation I am building is more implementor focused

The engine_api has both a user facing https://ethereum.github.io/beacon-APIs/ and a developer-facing specification which goes into how RPC endpoints should behave which is more developer focused

[bhartnett]

Poke is currently not part of the state network. Not sure if this should be mentioned.

[KolbyML]

Poke is currently not part of the state network. Not sure if this should be mentioned.

good point I will remove it 🚀

[pipermerriam]

I was under the impression that we could indeed poke the content, since we are effectively building up a proof by walking the trie... Couldn't the node that is doing the trie walk push the content into the nodes that should have had it but didn't after retrieval?

[KolbyML]

I was under the impression that we could indeed poke the content, since we are effectively building up a proof by walking the trie... Couldn't the node that is doing the trie walk push the content into the nodes that should have had it but didn't after retrieval?

You are right, so I will keep poke listed here

[bhartnett]

Actually poke isn't possible in portal_stateGetContent because we don't have the proof needed in order to build an offer in this context. The higher level rpc endpoints such as eth_getBalance will have this proof from walking the trie but portal_stateGetContent doesn't have a proof parameter and may get called/used in other contexts where we are not walking the trie. Sure, you could in theory walk the trie for every bit a state content being looked up but that would be very slow due to the many network requests potentially required.

[morph-dev]

We could add optional proof field to the portal_stateGetContent. If provided, it can be used together with content obtained from the network to generate content that can be poked.

This would technically make get_content for State different then the one from History and Beacon. But it actually is not that different, because for Beacon and History, you don't need any additional data in order to create poke content item.

I didn't think extensively about it, but it feels like we would have similar need for Transaction and Verkle networks (when added). So State wouldn't be the only network that will need this.

Side note: I'm on the fence if the field should be optional or not. I'm not sure in what situation one would have content key, but not have a proof for it. But on the other hand, I don't want to make API too strict.

[bhartnett]

Yes we could add that parameter for state network then the poke would be possible. My thoughts are that it should be optional so that state content can still be fetched for other purposes such as debugging if and when needed.

[morph-dev]

I don't think we should make API decisions with debugging/testing use case in mind. If we need something specific for debugging, those can be added separately (e.g. portalDebug_stateGetContent) and they can be:

1. documented to behave the same as the original request but skipping validation
2. enabled/disabled with a flag

But, if we have legitimate use case when we are requesting data but don't already have proof available, then field should of course be optional. I'm just not sure if such use case exists

[pipermerriam]

Since we sometimes can poke the content, we should just specify this as a SHOULD with that context so that in the event that the call if poke-able it should be poked, otherwise, if it's unanchored content, it shouldn't.

[bhartnett]

We have recently reviewed and updated the validation of the state portal JSON-RPC endpoints and this is what we implemented based on what intuitively made sense:

• All content key parameters are now validated by decoding and checking that the content key is a valid type.
• All content values (offers) received as parameters are validated using local validations without network lookups. For the state network this means skipping the part that looks up the block header to get the state root but we can still validate the structure of the proofs and that the nodeHash in the content key matches the proof etc.
• All content fetched from the network for findContent endpoints is validated using local validation, in the case of the state network we decode and then check that the hash in the content key matches the hash of the returned value.
• Some endpoints store in the local db and others don't.
  • stateStore and stateLocalContent only touch the local db as expected.
  • stateRecursiveFindContent (being renamed to stateGetContent) and stateGossip store in the local db as we are thinking of these like 'high level' endpoints.
  • stateFindContent and stateOffer both query or send to a specific enr and therefore we opted to not store in the db and treat these as low level endpoints.

This was just our initial intuition of what seems like be a good implementation and perhaps the spec could head in this direction. I think we need to be clear what we mean by validation in the spec as well because the ones that can be done without network look ups should probably still be required by the spec.

[kdeme]

The engine_api has both a user facing https://ethereum.github.io/beacon-APIs/ and a developer-facing specification which goes into how RPC endpoints should behave which is more developer focused

I assume you meant to link https://github.com/ethereum/execution-apis/tree/main/src/engine.
But yes, good point on user vs implementer docs, I didn't look at it with this differentiation. Perhaps that should be made clear also.

But I do think that in some cases you can just write it differently in the "users" specification because it is still very useful information to the user in the end.
E.g.
Implementer doc/spec would say: "Data MUST be validated by its hash before returning"
User doc/spec: "Returned data is validated by its hash"

By the way, https://ethereum.github.io/beacon-APIs/ is quite detailed also, especially if you start looking into the validator API.