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

Make fastest network finding more efficient #43

Closed
gentlementlegen opened this issue Aug 8, 2024 · 8 comments · Fixed by #45
Closed

Make fastest network finding more efficient #43

gentlementlegen opened this issue Aug 8, 2024 · 8 comments · Fixed by #45

Comments

@gentlementlegen
Copy link
Member

Instead of trying to resolve all the promises to see which network is the fastest, it would be more efficient to return the first one that resolves the network call successfully as it is probably the fastest.

          @Keyrxng @0x4007 I changed the logic a bit so instead of naively removing the first RPC of the list, the failing RPC is actually removed.

This also made me wonder, why do we test every endpoint when we are only interested in the fastest one, aka the first one to successfully fulfill the response? If we have to wait for slow RPCs, for example one that take 5 seconds to complete, doesn't it defeat the purpose of the function?

Originally posted by @gentlementlegen in #33 (comment)

Copy link

ubiquibot bot commented Aug 8, 2024

@gentlementlegen, You are not allowed to add Time: <2 Hours

@gentlementlegen gentlementlegen self-assigned this Aug 8, 2024
Copy link

ubiquibot bot commented Aug 8, 2024

! No permission to set labels

Copy link

ubiquibot bot commented Aug 8, 2024

@gentlementlegen, You are not allowed to add Priority: 2 (Medium)

Copy link

ubiquibot bot commented Aug 8, 2024

! No permission to set labels

@0x4007
Copy link
Member

0x4007 commented Aug 16, 2024

@gentlementlegen I added the labels for you 😄

Copy link

ubiquibot bot commented Aug 18, 2024

@gentlementlegen the deadline is at 2024-08-18T11:08:57.568Z

@gentlementlegen
Copy link
Member Author

@0x4007 It seems that closing issues automatically does not trigger the conversation rewards. Maybe this is because the actor is a bot so it gets skipped by the kernel.

Copy link
Contributor

ubiquity-os bot commented Aug 23, 2024

[ 136.6 WXDAI ]

@gentlementlegen
Contributions Overview
View Contribution Count Reward
Issue Task 1 100
Issue Specification 1 36.6
Issue Comment 1 0
Review Comment 6 0
Conversation Incentives
Comment Formatting Relevance Reward
Instead of trying to resolve all the promises to see which netwo…
36.6
content:
  p:
    count: 122
    score: 1
  em:
    count: 6
    score: 0
wordValue: 0.1
formattingMultiplier: 3
1 36.6
@0x4007 It seems that closing issues automatically does not trig…
5.8
content:
  p:
    count: 29
    score: 1
wordValue: 0.2
formattingMultiplier: 1
- -
Resolves #43 QA (used with the newly created function): https:…
0
content:
  p:
    count: 10
    score: 1
wordValue: 0
formattingMultiplier: 0
0.2 -
It returns a single item not an array, what can I count?
0
content:
  p:
    count: 12
    score: 1
wordValue: 0.2
formattingMultiplier: 0
1 -
Simply to avoid always using the same providers in the same orde…
0
content:
  p:
    count: 46
    score: 1
wordValue: 0.2
formattingMultiplier: 0
1 -
I have no idea, not of my making, but my guess would be to avoid…
0
content:
  p:
    count: 22
    score: 1
wordValue: 0.2
formattingMultiplier: 0
1 -
@Keyrxng I guess I can remove the proxy, although it is removabl…
0
content:
  h2:
    count: 55
    score: 1
  p:
    count: 16
    score: 1
wordValue: 0.2
formattingMultiplier: 0
1 -
@Keyrxng the tests and overall functions were unreliable when no…
0
content:
  p:
    count: 102
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 0
1 -

[ 2.18 WXDAI ]

@0x4007
Contributions Overview
View Contribution Count Reward
Issue Comment 1 0.08
Review Comment 3 2.1
Conversation Incentives
Comment Formatting Relevance Reward
@gentlementlegen I added the labels for you 😄
0.8
content:
  p:
    count: 8
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.1 0.08
Maybe also count the results, it should be one.
0.9
content:
  p:
    count: 9
    score: 1
wordValue: 0.1
formattingMultiplier: 1
1 0.9
What's the purpose of shuffling the array?
0.7
content:
  p:
    count: 7
    score: 1
wordValue: 0.1
formattingMultiplier: 1
1 0.7
I wonder why two underscores.
0.5
content:
  p:
    count: 5
    score: 1
wordValue: 0.1
formattingMultiplier: 1
1 0.5

[ 31.9 WXDAI ]

@Keyrxng
Contributions Overview
View Contribution Count Reward
Review Comment 3 31.9
Conversation Incentives
Comment Formatting Relevance Reward
Gave the function a try in `script-test.ts` and it looks…
6.8
content:
  p:
    count: 67
    score: 1
  code:
    count: 1
    score: 1
  img:
    count: 1
    score: 0
wordValue: 0.1
formattingMultiplier: 1
1 6.8
I like this idea we should have had something like this from the…
9.8
content:
  p:
    count: 23
    score: 1
  ul:
    count: 74
    score: 0
  li:
    count: 74
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.1
formattingMultiplier: 1
1 9.8
I think it makes sense to have this return only a non-proxy sinc…
15.3
content:
  p:
    count: 146
    score: 1
  code:
    count: 7
    score: 1
  pre:
    count: 6
    score: 0
wordValue: 0.1
formattingMultiplier: 1
1 15.3

0x4007 pushed a commit that referenced this issue Oct 10, 2024
…equest-event

chore: update knip configuration in the template to run on pull_request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants