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

fix: index-provider: change connection initiation direction between daemon and market #9627

Merged
merged 1 commit into from
Nov 23, 2022
Merged

fix: index-provider: change connection initiation direction between daemon and market #9627

merged 1 commit into from
Nov 23, 2022

Conversation

LexLuthr
Copy link
Contributor

@LexLuthr LexLuthr commented Nov 10, 2022

Related Issues

#9626

Proposed Changes

We need to change the direction of libp2p connection init. Rather than dialling from market to fullNode, we should dial from fullNode to market. Thus removing the requirement to have libp2p exposed.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@LexLuthr LexLuthr linked an issue Nov 10, 2022 that may be closed by this pull request
15 tasks
@LexLuthr LexLuthr marked this pull request as ready for review November 20, 2022 05:44
@LexLuthr LexLuthr requested a review from a team as a code owner November 20, 2022 05:44
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

This makes sense.

We now don't need to gen faddrs above, also should fix the log below.

@LexLuthr
Copy link
Contributor Author

This makes sense.

We now don't need to gen faddrs above, also should fix the log below.

@magik6k As per my understanding, we still need to protect the connection from both sides. faddrs is being used for that.
Can you confirm which log do I need to change?

@magik6k
Copy link
Contributor

magik6k commented Nov 23, 2022

  • Log - the log.Debugw("successfully connected to full node and asked it protect indexer provider peer conn", "fullNodeInfo", faddrs.String(), "peerId", marketsPeerID) one - tho reading this code today it probably doesn't make sense to change it.
  • Oh didn't notice the conn protection part

@magik6k magik6k merged commit 0b85c15 into filecoin-project:master Nov 23, 2022
@LexLuthr LexLuthr deleted the fix/idx-provdr-connection branch November 23, 2022 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove exposing full node libp2p address as a requirement for index-provider
2 participants