Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

IPC-38: IPLD Resolver smoke test #64

Merged
merged 18 commits into from
Mar 7, 2023
Merged

IPC-38: IPLD Resolver smoke test #64

merged 18 commits into from
Mar 7, 2023

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Mar 3, 2023

Closes consensus-shipyard/ipc#464

Smoke test exercising the simplest possible scenario of starting some nodes, bootstrapping from the 1st, putting some content in the store of the 2nd, resolving by the 3rd.

Example run:

RUST_LOG=debug cargo test -p ipc_ipld_resolver --test smoke

There are some non-ideal things left in the PR, to be fixed later:

  • it would be nice to switch to structured logging
  • it would be great not to have to rely on sleep in the tests

But I think this is not bad to show that something works, with all the fixes that needed for it to happen, it's not a small PR.

@aakoshh aakoshh changed the base branch from main to ipc-37-service March 3, 2023 16:39
@aakoshh aakoshh marked this pull request as ready for review March 3, 2023 23:29
/// This seems to be the only way, because Kademlia rightfully treats
/// incoming connections as ephemeral addresses, but doesn't have an
/// alternative exchange mechanism.
pub fn add_identified(&mut self, peer_id: &PeerId, info: &Info) {
Copy link
Contributor Author

@aakoshh aakoshh Mar 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Buffer these until bootstrapping is finished.

Created https://github.com/consensus-shipyard/ipc-agent/issues/70

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #74

Base automatically changed from ipc-37-service to main March 6, 2023 09:47
@aakoshh aakoshh force-pushed the ipc-38-resolver-test branch 2 times, most recently from 4a4d0db to cd520e4 Compare March 6, 2023 10:13
Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! It is so exciting that this works 🙌


// Wait a little for the gossip to spread and peer lookups to happen, then another round of gossip.
// TODO: Wait on some condition instead of sleep.
tokio::time::sleep(Duration::from_secs(2)).await;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This have been shown to be a recurring issue when testing gossipsub-related protocols. >1s sync delay (which is the default heartbeat time) is the easy one, but IIRC we can probably listen or poll the libp2p host to see if it is bootrstapped. I don't know if this can be done easily in Rust, I did this once in Go.

(We can do something similar with the bootstrap delay above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the tips. I can also add more events to show when things have progressed, and I have seen examples in the libp2p tests of a combination of a polling function and a timeout.

@aakoshh aakoshh merged commit 2936355 into main Mar 7, 2023
@aakoshh aakoshh deleted the ipc-38-resolver-test branch March 7, 2023 10:03
@aakoshh aakoshh mentioned this pull request Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IPLD Resolver: Integration Test
2 participants