docs: P2P network ADR 001 #66

rach-id · 2023-01-25T18:21:19Z

Overview

Closes #4

Checklist

New and updated code has appropriate documentation
New and updated code has new and/or updated testing
Required CI checks are passing
Visual proof for any user facing features like CLI or documentation updates
Linked issues closed with keywords

rahulghangas

LGTM, minor nits

rahulghangas · 2023-01-30T11:47:38Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+**Producer**: Orchestrator, which will be signing attestations:
+
+- Sign an attestation
+- Create the attestation confirm struct


Maybe initialize?

rahulghangas · 2023-01-30T11:48:27Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+- Create the attestation confirm struct
+- Post it to the blockstore
+- Create a `SignatureIndex`
+- Put the Signature index in the DHT.


Either SignatureIndex if we want to explicitly refer to type, or signature index (small case)

evan-forbes

I don't want to block too much on this ADR as getting the implementation out the door is a priority, but we might want to slowly flesh this out over time as I do think writing these things down is a good forcing function to really nail down the reasoning for the approach that we've so far only discussed in calls. that would also give us more time to add details to the implementation portion.

evan-forbes · 2023-01-30T12:22:45Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+
+### Positive
+
+- Use a well tested/maintained project, libP2P implementation of DHT.


[nit non blocking]
we could perhaps be a bit clearer here, or I'm a tad confused. Is this a benefit relative to what exactly? would we implement our own DHT using the other option? alternatively, here's a suggestion for better wording

Suggested change

- Use a well tested/maintained project, libP2P implementation of DHT.

- Use a well tested/maintained implementation of DHT.

evan-forbes · 2023-01-30T12:26:40Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+
+### Validators
+
+To add an extra layer of security when exchanging the data, we will define validators which will validate the confirms before storing/querying them:


[nit]
I'm not sure this adds any level of security, but perhaps that's a definitions thing. What do we mean here by "security"?

before storing/querying

the wording was a bit confusing here imo. We run these after we query them, correct? or perhaps during, but I don't think we run these before

evan-forbes · 2023-01-30T12:31:55Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+func (vv ValsetValidator) Validate(key string, value []byte) error {
+  // TODO Should verify that the valset is valid, i.e. running stateless checks on it.
+  // The checks should include:
+  // - Correct signature verification
+  // - Correct fields checks. Example, checking if an address field as a correctly formatted address.
+  return nil
+}
+
+func (vv ValsetValidator) Select(key string, values [][]byte) (int, error) {
+  // TODO Should run the same stateless checks as the `Validate` function to avoid querying
+  // faulty values.
+  return 0, nil
+}
+```


[non blocking question]
what happens to a peer if these return an error?

[nit]
the naming of these seem redundant or confusing because of the double use of the word validator (valset and Validator) which mean very different things. I would prefer that we find a different name, but not going to block. I feel like we don't have to name something a validator just because that the role it plays in libp2p. The method names are already descriptive enough imo, so we can use decorator, signatureChecker, verifier etc

evan-forbes · 2023-01-30T12:34:00Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+  - A DAG-syncer: which syncs DAG nodes between nodes and publishes new ones. IPFS will be used for this.
+  - A broadcaster: which sends messages to all the peers. This will be LibP2P PubSub.
+- Limitations:
+  - Ever-growing DAG: shouldn't be a problem in our case since we only have small unrelated key values.


as discussed in the initial call, we would definitely prune, each peer just has to prune the same

evan-forbes · 2023-01-30T12:36:17Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+  - Ever-growing DAG: shouldn't be a problem in our case since we only have small unrelated key values.
+  - Merkle clock sorting: having access to a logical clock to sort events as they happen. This shouldn't cause us any issue since the signatures are not order dependent.
+  - TBD.
+- We should take a deeper look at the repo [go-ds-crdt](https://github.com/ipfs/go-ds-crdt) as there isn't much activity:


might want to reword this a bit to describe what is meant here in a clearer way. Also, while that repo might not have a lot of usage or issues, it is using standard libp2p components which have been tested heavily. Its more like a configured version of ipfs-lite, not unlike what we're suggesting we do here

evan-forbes · 2023-01-30T12:46:05Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+confirm, err := exchange.GetBlock(ctx, index.CID)
+```
+
+### Limitations


this title feels out of place

evan-forbes · 2023-01-30T13:06:40Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+- `orchestrator`: orchestrator address
+- `type`: data commitment confirm or valset confirm
+
+The issue with using a P2P network is that there is no way of knowing the CID of a signature beforehand. So, relayers will not be able to get the confirms directly from the network.


there's no way of knowing the hash before hand, but if we define a custom CID format (similar to the plugin), then we can know the CID ahead of time!

also, mind clarifying why we can't know the hash ahead of time to readers of this document?

evan-forbes · 2023-01-30T13:08:12Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+
+## Possible solutions
+
+The standard solution is to use Libp2p kadDHT implementation for discovery and routing, then using a Bitswap exchange to publish attestation signatures and get them using CIDs. However, in our case, the CIDs will be unknown until the moment of signature creation making it impossible to rely on Bitswap alone. So, we will need to use another mechanism to know the CIDs or to get them.


Suggested change

The standard solution is to use Libp2p kadDHT implementation for discovery and routing, then using a Bitswap exchange to publish attestation signatures and get them using CIDs. However, in our case, the CIDs will be unknown until the moment of signature creation making it impossible to rely on Bitswap alone. So, we will need to use another mechanism to know the CIDs or to get them.

The standard solution is to use libp2p kadDHT implementation for discovery and routing, then using a Bitswap exchange to publish attestation signatures and get them using CIDs. However, in our case, the CIDs will be unknown until the moment of signature creation making it impossible to rely on Bitswap alone. So, we will need to use another mechanism to know the CIDs or to get them.

we used a few different formats of libp2p, such as libP2P, and should likely just stick to a single one.

evan-forbes · 2023-01-30T13:09:49Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+- Consumers will have to know when to query the network:
+  - This won't be an issue since we can define a delay to start querying: the time when the attestation request was created in the state machine + a few minutes


if this isn't an issue, then we might not want to list it as a limitation. Also, don't we have to know when to query for all approaches?

evan-forbes · 2023-01-30T13:11:09Z

docs/architecture/adr-001-initial-QGB-p2p-network.md

+
+### Option 4: IPNS
+
+Instead of using the DHT for storing the indexes, we could use IPNS where each peer would publish its list of signatures, and upon every signature, update what it's publishing.


I think we could flesh this out a bit more, as I'm not sure how this works still.

evan-forbes · 2023-12-04T13:01:40Z

I think we can close this as unplanned since I don't think its worth finishing this any longer

docs: add P2P network ADR 1

60678a2

rach-id added documentation Improvements or additions to documentation p2p p2p network related labels Jan 25, 2023

rach-id requested a review from rahulghangas January 25, 2023 18:21

rach-id requested a review from evan-forbes as a code owner January 25, 2023 18:21

rach-id self-assigned this Jan 25, 2023

rach-id added 2 commits January 25, 2023 19:45

chore: cosmetics

81f8be1

docs: use spaces instead of tabs

2b85412

This was referenced Jan 26, 2023

Add attestation confirms marshal/unmarshal methods #69

Closed

Implement validators for the DHT values #74

Closed

Add an initial DHT implementation #78

Closed

rahulghangas approved these changes Jan 30, 2023

View reviewed changes

evan-forbes reviewed Jan 30, 2023

View reviewed changes

This was referenced Jan 31, 2023

Orchestrator address <> EVM address mapping #89

Closed

EPIC: QGB V2 MVP tracker #92

Closed

Remove the orchestrator address from the validator celestiaorg/cosmos-sdk#299

Closed

rach-id mentioned this pull request Feb 21, 2023

docs: hoist ADR status celestiaorg/celestia-app#1407

Merged

rach-id mentioned this pull request Apr 3, 2023

QGB v2 Beta tracker #203

Open

30 tasks

rach-id marked this pull request as draft September 19, 2023 10:56

rach-id closed this Dec 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: P2P network ADR 001 #66

docs: P2P network ADR 001 #66

rach-id commented Jan 25, 2023

rahulghangas left a comment

rahulghangas Jan 30, 2023

rahulghangas Jan 30, 2023

evan-forbes left a comment

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes Jan 30, 2023

evan-forbes commented Dec 4, 2023


		### Positive

		- Use a well tested/maintained project, libP2P implementation of DHT.

	- Use a well tested/maintained project, libP2P implementation of DHT.
	- Use a well tested/maintained implementation of DHT.


		### Validators

		To add an extra layer of security when exchanging the data, we will define validators which will validate the confirms before storing/querying them:


		## Possible solutions

		The standard solution is to use Libp2p kadDHT implementation for discovery and routing, then using a Bitswap exchange to publish attestation signatures and get them using CIDs. However, in our case, the CIDs will be unknown until the moment of signature creation making it impossible to rely on Bitswap alone. So, we will need to use another mechanism to know the CIDs or to get them.

		- Consumers will have to know when to query the network:
		- This won't be an issue since we can define a delay to start querying: the time when the attestation request was created in the state machine + a few minutes


		### Option 4: IPNS

		Instead of using the DHT for storing the indexes, we could use IPNS where each peer would publish its list of signatures, and upon every signature, update what it's publishing.

docs: P2P network ADR 001 #66

docs: P2P network ADR 001 #66

Conversation

rach-id commented Jan 25, 2023

Overview

Checklist

rahulghangas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evan-forbes left a 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

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

evan-forbes commented Dec 4, 2023