Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: console wallet use dns seeds #6034

Merged
merged 3 commits into from
Dec 12, 2023

Conversation

SWvheerden
Copy link
Collaborator

Description

Console wallet use dns seeds

Motivation and Context

The console wallet should use the DNS seed, and not just the seeds in the config file

How Has This Been Tested?

Manual

Copy link

github-actions bot commented Dec 12, 2023

Test Results (CI)

1 264 tests   1 264 ✔️  12m 2s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit 0f035ab.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Dec 12, 2023
Copy link

github-actions bot commented Dec 12, 2023

Test Results (Integration tests)

28 tests   28 ✔️  11m 52s ⏱️
11 suites    0 💤
  2 files      0

Results for commit 0f035ab.

♻️ This comment has been updated with latest results.

@stringhandler
Copy link
Collaborator

Nice find

Copy link
Contributor

@brianp brianp left a comment

Choose a reason for hiding this comment

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

I think I want to test this.

but a clarification if you will:

should use the DNS seed, and not just the seeds in the config file

and not just

So it should use both? Because the change swings the pendulum, to ignore the config seeds, and use whatever the PeerDB already has.
At what point did it fetch DNS seeds?

@SWvheerden
Copy link
Collaborator Author

ld use the DNS seed, and not just the seeds in the config file

and not just

it should use both:

let peers = Self::try_parse_seed_peers(&self.seed_config.peer_seeds)?;

Copy link
Contributor

@brianp brianp left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the response.

Tested locally:
Created new wallet on esme network. Removed the peers from the config. Deleted the dht, and peer db. Restarted the wallet and it started finding peers instantly, found and connected to a base node, and carried on.

👍🏻

Ack (sans ut)

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Dec 12, 2023
@SWvheerden SWvheerden merged commit b194954 into tari-project:development Dec 12, 2023
13 of 14 checks passed
@SWvheerden SWvheerden deleted the sw_fix_wallet_dns branch December 12, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants