feat: add index provider library #143

NikolasHaimerl · 2025-04-01T13:28:55Z

This PR uses the newly released library to fetch index provider peer ids.

Related to
CheckerNetwork/index-provider-peer-id#10
CheckerNetwork/roadmap#250
CheckerNetwork/roadmap#244

Co-authored-by: Miroslav Bajtoš <[email protected]>

…/filecoin-station/deal-observer into nhaimerl-telemetry-payload-state

backend/bin/deal-observer-backend.js

backend/test/resolve-payload-cids.test.js

backend/bin/deal-observer-backend.js

backend/lib/resolve-payload-cids.js

Co-authored-by: Julian Gruber <[email protected]>

forgot the one change request

bajtos

The change looks pretty good, let's improve the details.

backend/bin/deal-observer-backend.js

backend/lib/resolve-payload-cids.js

backend/lib/rpc-service/service.js

NikolasHaimerl · 2025-04-02T12:12:34Z

@bajtos it seems like the GLIF token we use for RPC calls cannot be used as well for the smart contract calls: https://github.com/CheckerNetwork/spark-deal-observer/actions/runs/14218917560/job/39841932436?pr=143#step:7:125

Should I use the RPC_AUTH from spark checker for the smart contract calls instead?
I am not 100% sure what the difference in those tokens is but it seems that if I use the one from spark-checker the smart contract calls work.

NikolasHaimerl · 2025-04-02T14:30:08Z

Re: GLIF API token.

According to https://github.com/CheckerNetwork/spark-deal-observer/settings/secrets/actions, this repository uses the same Glif API token we use in all other GH repositories, too (it's an org-wide secret).

Places to check:
* Does the CI workflow propagate the secret to the env vars?

* Does your test code read the API token from the correct env var?
I suggest adding an assertion to verify that the API token is set so that tests fail early with a helpful error message if the token is not provided.

It seems like the GLIF_TOKEN is not picked up although it is set as a secret for the repo. I'll have a look.

backend/lib/resolve-payload-cids.js

Co-authored-by: Julian Gruber <[email protected]>

NikolasHaimerl · 2025-04-02T15:03:14Z

@bajtos @juliangruber authentication issue should be fixed now.

backend/lib/resolve-payload-cids.js

juliangruber · 2025-04-03T10:32:12Z

Blocked by #145

bajtos · 2025-04-04T13:38:50Z

Blocked by #145

I disagree.

The current version on the main branch does not configure any timeouts for RPC API calls, see https://github.com/search?q=repo%3ACheckerNetwork%2Fspark-deal-observer%20timeout&type=code
This PR is a critical piece we need to ship to support Curio DDO deals. We have been working on that feature for 3 weeks now. I want to finish it ASAP.
Requiring this PR to introduce timeouts for RPC API calls is IMO a scope creep delaying an important feature from being shipped.

I propose the following steps forward:

Land this PR as soon as we have a version that works and with an acceptable code quality
Add timeouts and better TypeScript types as part of this milestone, but only after we finish other tasks required to allow Spark to test Curio DDO deals.

bajtos · 2025-04-04T13:40:35Z

backend/lib/resolve-payload-cids.js

+import {
+  getIndexProviderPeerId,
+  MINER_TO_PEERID_CONTRACT_ADDRESS, MINER_TO_PEERID_CONTRACT_ABI
+// @ts-ignore


Is there any follow-up issue (or a check-list item in an existing issue) to remind us that we need to remove this ts-ignore directive once index-provider-peer-id includes type definitions?

There is already a PR for it closing the issue that is tracking this: CheckerNetwork/index-provider-peer-id#21

I am confused. After you land that pull request and auto-close the issue, who is going to edit backend/lib/resolve-payload-cids.js in this repository to remove the // @ts-ignore line?

After the pull request CheckerNetwork/index-provider-peer-id#21 is merged I can update the npm package version and then I should be able to remove the // @ts-ignore in all three repos.

Excellent!

My question at the top of this thread was about ensuring this cleanup task is not forgotten.

Is there any follow-up issue (or a check-list item in an existing issue) to remind us that we need to remove this ts-ignore directive once index-provider-peer-id includes type definitions?

I created subtasks in the original issue to keep track

backend/lib/resolve-payload-cids.js

juliangruber · 2025-04-08T09:12:17Z

backend/test/resolve-payload-cids.test.js

+  const getPeerId = async (/** @type {number} */ minerId) => {
+    const peerId = minerPeerIds.get(`f0${minerId}`)?.PeerId
+    if (!peerId) {
+      throw new Error(`Peer ID not found for miner ID: ${minerId}`)
+    }
+    return Promise.resolve({ peerId, source: 'TEST' })
+  }


Either remove the async keyword or make it return { peerId, source.. }, having both is redundant (you're creating a promise that's resolving with a promise that's resolving with the value).

Addressed in 330d290

backend/test/resolve-payload-cids.test.js

Co-authored-by: Julian Gruber <[email protected]>

Nikolas Haimerl and others added 13 commits February 17, 2025 16:32

add count of payload cid state

de9a405

Update backend/bin/deal-observer-backend.js

ec6d5d4

Co-authored-by: Miroslav Bajtoš <[email protected]>

Update backend/lib/look-up-payload-cids.js

fa6cdf2

Co-authored-by: Miroslav Bajtoš <[email protected]>

merged with main

9a980b6

Merge branch 'nhaimerl-telemetry-payload-state' of https://github.com…

5221f01

…/filecoin-station/deal-observer into nhaimerl-telemetry-payload-state

changed testing

a52a047

moved default active deal

3711e62

resolve PR threads

de7ad74

changed storage function

ae43469

fmt

711af9d

add index lookup library

0e59ebb

merged with main

980807b

fix tests

f1c6b2b

NikolasHaimerl commented Apr 1, 2025

View reviewed changes

backend/bin/deal-observer-backend.js Outdated Show resolved Hide resolved

NikolasHaimerl commented Apr 1, 2025

View reviewed changes

backend/test/resolve-payload-cids.test.js Show resolved Hide resolved

cleanup

cb5473e

NikolasHaimerl marked this pull request as ready for review April 1, 2025 15:50

NikolasHaimerl requested a review from bajtos as a code owner April 1, 2025 15:50

NikolasHaimerl requested a review from juliangruber April 1, 2025 15:50

NikolasHaimerl mentioned this pull request Apr 1, 2025

Support smart-contract-based mapping of MinerID to IndexProviderPeerID CheckerNetwork/roadmap#250

Closed

7 tasks

juliangruber requested changes Apr 2, 2025

View reviewed changes

backend/bin/deal-observer-backend.js Show resolved Hide resolved

backend/bin/deal-observer-backend.js Outdated Show resolved Hide resolved

backend/lib/resolve-payload-cids.js Outdated Show resolved Hide resolved

NikolasHaimerl changed the title ~~feat: add index probier library~~ feat: add index provider library Apr 2, 2025

Update backend/bin/deal-observer-backend.js

7f3944a

Co-authored-by: Julian Gruber <[email protected]>

juliangruber previously approved these changes Apr 2, 2025

View reviewed changes

bajtos requested changes Apr 2, 2025

View reviewed changes

backend/bin/deal-observer-backend.js Outdated Show resolved Hide resolved

backend/bin/deal-observer-backend.js Outdated Show resolved Hide resolved

backend/lib/resolve-payload-cids.js Outdated Show resolved Hide resolved

backend/lib/rpc-service/service.js Show resolved Hide resolved

Nikolas Haimerl added 3 commits April 2, 2025 13:48

resolve PR threads

a086433

resolve PR threads

b76ecdb

fmt

08d80c2

removed default value

d9af29c

add secret to workflow

4fc01e3

juliangruber requested changes Apr 2, 2025

View reviewed changes

backend/lib/resolve-payload-cids.js Outdated Show resolved Hide resolved

Nikolas Haimerl and others added 3 commits April 2, 2025 16:59

add secret to workflow

af8c6e2

Update backend/lib/resolve-payload-cids.js

862137a

Co-authored-by: Julian Gruber <[email protected]>

fmt

a23e643

NikolasHaimerl requested review from bajtos and juliangruber April 2, 2025 15:04

Nikolas Haimerl added 2 commits April 2, 2025 17:05

remove glif token during deployment

5f53092

fix async test

dc56ecd

bajtos reviewed Apr 3, 2025

View reviewed changes

backend/lib/resolve-payload-cids.js Show resolved Hide resolved

NikolasHaimerl mentioned this pull request Apr 3, 2025

Add timout for fetching peer id #145

Closed

4 tasks

juliangruber added the blocked label Apr 3, 2025

NikolasHaimerl mentioned this pull request Apr 3, 2025

feat: add index provider library CheckerNetwork/spark-checker#129

Merged

NikolasHaimerl requested a review from bajtos April 4, 2025 08:08

bajtos approved these changes Apr 4, 2025

View reviewed changes

juliangruber requested changes Apr 8, 2025

View reviewed changes

NikolasHaimerl and others added 3 commits April 8, 2025 11:43

Update backend/lib/resolve-payload-cids.js

33ffd18

Co-authored-by: Julian Gruber <[email protected]>

Update backend/lib/resolve-payload-cids.js

dc18bc5

Co-authored-by: Julian Gruber <[email protected]>

removed redundant async

330d290

NikolasHaimerl requested a review from juliangruber April 8, 2025 09:53

juliangruber approved these changes Apr 8, 2025

View reviewed changes

Merge branch 'main' into nhaimerl-add-index-probier-library

0b92f2c

NikolasHaimerl enabled auto-merge (squash) April 8, 2025 10:26

NikolasHaimerl merged commit 7a2a314 into main Apr 8, 2025
8 checks passed

NikolasHaimerl deleted the nhaimerl-add-index-probier-library branch April 8, 2025 10:28

feat: add index provider library #143

feat: add index provider library #143

Uh oh!

Conversation

NikolasHaimerl commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bajtos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NikolasHaimerl commented Apr 2, 2025

Uh oh!

NikolasHaimerl commented Apr 2, 2025

Uh oh!

Uh oh!

NikolasHaimerl commented Apr 2, 2025

Uh oh!

Uh oh!

juliangruber commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bajtos commented Apr 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NikolasHaimerl commented Apr 1, 2025 •

edited

Loading

juliangruber commented Apr 3, 2025 •

edited

Loading