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: improved origins filtering #123

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

feat: improved origins filtering #123

wants to merge 15 commits into from

Conversation

olizilla
Copy link
Contributor

@olizilla olizilla commented Mar 14, 2023

kubo provides it's own multiaddrs as origins on pin service requests when it has the blocks for a pin request locally. This PR aims to filter out unusable multiaddrs early, and make a best effort to convert them into something we can connect to.

  • grab p2p addrs from origins if we can.
  • limit origins to 10, just so we dont accept unboundedly long origins lists

License: MIT

WIP

- switch to an sqs lib that polls for new messages concurrently rather than in batches
- rewrite pickup worker so we can compose it out of single-responsibilty pieces instead of having to pass through the giant config ball.

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
test the api is available when start is called.

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
fixes #101

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
2 mins is still timeout after node restarts

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
- grab p2p addrs from origins if we can.
- limit origins to 10, just so we dont accept unboundedly long origins lists

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
Base automatically changed from concurrently to main March 14, 2023 16:12
@vasco-santos
Copy link
Member

vasco-santos commented Mar 15, 2023

@olizilla mind rebasing this before review please? 🙏🏼 Most of this diff is already in #main and makes quite difficult to review changes

@olizilla
Copy link
Contributor Author

Sorry! done!

note, i'd encourage merging from main rather than rebase these days as:...

Warning: Because changing your commit history can make things difficult for everyone else using the repository, it's considered bad practice to rebase commits when you've already pushed to a repository."
– https://docs.github.com/en/get-started/using-git/about-git-rebase

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Non blocking feedback. But thing would be good to be addressed

return input
.split(',')
.filter(isMultiaddr)
.filter(hasPublicIpAddress)
Copy link
Member

Choose a reason for hiding this comment

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

No more usage of this function. Looks like we can remove it

import { findUsableMultiaddrs } from '../basic/helper/multiaddr.js'

const fixture = [
'/ip4/127.0.0.1/tcp/4001/p2p/12D3KooWADjHf2kyANQodg9z5sSdX4bGEMbWg7ojwu6SCyDAMtzN',
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a dns4 address just to make sure everything is working as expected for non hasBogonIpAddress?

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.

None yet

2 participants