message wrapping in protocols package is not devp2p compatible

[zelig]

protocols package uses message wrapping to pass context between nodes in order to facilitate tracing of requests across nodes.

This wrapping however is not compatible with devp2p so cannot be used in protocols supporting cross-client communication.

Such context passing needs to be thought through and we should have a clear roadmap of making this feature debug only.

[nonsense]

this was a design decision at the time we introduced tracing. it has always been the case that the protocols package is used only for swarm protocols (which is also why it lives in the ethersphere repo, and not in the ethereum/go-ethereum repo. we also agreed at the time that all protocols we write should be traceable (which is not really the case - think pss).

see #764 which explicitly says that we want nodes to be compatible even if tracing is enabled or disabled.

---

now that we need to include existing protocols outside of swarm (for example eth/63 or others), one solution i can think of is to not use the protocols package (similar to pss), or move the context propagation to another level.

[zelig]

Context propagation is a debug feature, it should be used if the peers excplicitly allow it.
It should be also independent of using protocols package.
I think it is best to just make it conditional on a flag set in the protocol spec or in the handshake or ENR

[nonsense]

It is already a debug feature and only enabled if you allow it. How would you make tracing and non-tracing enabled nodes compatible otherwise?

…

On Wed, Jul 17, 2019, 8:45 AM Viktor Trón ***@***.***> wrote: Context propagation is a debug feature, it should be used if the peers excplicitly allow it. It should be also independent of using protocols package. I think it is best to just make it conditional on a flag set in the protocol spec or in the handshake or ENR — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1575?email_source=notifications&email_token=AAAMKG4NPSXBKXBCNVRJZ7TP725YDA5CNFSM4IEAYAC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2DGXPA#issuecomment-512125884>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAMKG4CEJPANSHC3NSQCFTP725YDANCNFSM4IEAYACQ> .

[acud]

@zelig why do we block on this? why can't we just build this protocol initially using p2p.Peer? is this such a big issue we need to block on? I'd really rather have an initial clean phase 0 merged rather than a PR that's gonna be left open for a month or two. Still integration work can be done in this topic. modularising debug functionalities is orthogonal IMHO.