feat: spark contract #377

LexLuthr · 2025-01-21T16:50:05Z

bajtos

I reviewed the design and have concerns about the feasibility/usability of the proposed solution. PTAL at my comments below.

market/ipni/ipni-provider/spark.go

market/ipni/spark/sol/README.md

bajtos · 2025-01-29T15:13:31Z

market/ipni/spark/sol/README.md

+### **Fetch Peer Data**
+
+```javascript
+const peerData = await contract.getPeerData(1000);
+console.log(peerData.peerID);
+console.log(peerData.signedMessage);
+```
+
+### Verify the signature for auth check
+
+```javascript
+import { HttpJsonRpcConnector, LotusClient } from "filecoin.js";
+import * as libp2pCrypto from "@libp2p/crypto"; // Ensure this library is installed
+import * as multihashes from "multihashes"; // For decoding PeerID
+
+/**
+ * Fetch miner's PeerID and verify the signature of a message.
+ * @param {string} minerAddress - Filecoin miner address
+ * @param {Buffer} message - The message that was signed
+ * @param {Buffer} signature - The signature to verify
+ */
+async function verifyMinerSignature(minerAddress, message, signature) {
+  try {
+    // Set up the Filecoin RPC client
+    const rpcUrl = "http://127.0.0.1:1234/rpc/v0"; // Replace with your Filecoin Lotus RPC URL
+    const connector = new HttpJsonRpcConnector({ url: rpcUrl });
+    const lotusClient = new LotusClient(connector);
+
+    // Step 1: Fetch miner info
+    const minerInfo = await lotusClient.state.minerInfo(minerAddress);
+
+    if (!minerInfo || !minerInfo.PeerId) {
+      throw new Error("Could not retrieve PeerID from miner info.");
+    }
+
+    // Extract PeerID
+    const { PeerId } = minerInfo;
+    console.log("Retrieved PeerID:", PeerId);
+
+    // Step 2: Decode PeerID to obtain the public key
+    const decodedPeerID = multihashes.decode(Buffer.from(PeerId, "base64"));
+    const publicKeyBytes = decodedPeerID.digest;
+
+    // Step 3: Generate a Libp2p public key object
+    const publicKey = await libp2pCrypto.keys.unmarshalPublicKey(publicKeyBytes);
+
+    // Step 4: Verify the signature
+    const isValid = await publicKey.verify(message, signature);
+
+    if (isValid) {
+      console.log("Signature is valid.");
+      return true;
+    } else {
+      console.log("Signature is invalid.");
+      return false;
+    }
+  } catch (error) {
+    console.error("Error during verification:", error.message);
+    throw error;
+  }
+}
+```


Thank you for including a detailed JavaScript example showing how to use this new smart contract.

IIUC, the messages (transactions) modifying the MinerID->PeerID mapping are signed using the libp2p PeerID as found in MinerInfo state, is that correct?

Does the smart contract verify those signatures and reject messages where the signature does not match, or is it necessary to always validate the signatures on the client trying to map miner to IPNI PeerID?

If client-side validation is required, how do you envision handling the situation when the miner changes their libp2p PeerID stored in the MinerInfo state?

Yes, they signed with existing on chain ID.

We don't verify the signature in contract. It is too expensive to do so. We only verify that sender is a control address for the miner address.

Hmm! For Curio it would be an easy fix. We can verify the signature and if it does not match then update the entry. This would ensure that if peerID changes then same is updated in the contract as well. Although, in real world, users won't be changing peerID. In fact, we don't have any options to update it in a cluster.

Fixed the peerID signature mismatch in Curio code. It checks and pushes a fix if required.

Perfect! Can you please document this requirement somewhere in the docs so that other implementers like Venus know about it?

This is not a library function. Not sure what you want me to document. This is implementation specific. Venus might want to do something else.

market/ipni/ipni-provider/spark.go

market/ipni/spark/sol/contracts/MinerPeerIDMapping.sol

market/ipni/spark/sol/README.md

bajtos · 2025-01-31T18:09:58Z

market/ipni/spark/sol/README.md

+
+/**
+ * Fetch miner's PeerID and verify the signature of a message.
+ * @param {string} minerAddress - Filecoin miner address


See, this line expects the minerAddress to be an f0 string.

IIUC, to fully validate the mapping, we need also need to check that minerAddress matches message.miner and minerInfo.PeerID matches message.peerID.

It would be great to include this step in the example code too. Maybe as a new function showing how to fully verify the mapping entry?

README will be updated once I have the tested the contract. README was written for the first version.

In #377 (comment), we were discussing the case where the message is signed using the PeerID found in the MinerInfo state.

In #377 (comment), we are discussing that the message should be signed by the PeerID found in the mapping.

As far as I understand, these two Peer IDs will be different in Curio, and that's the reason for introducing this smart contract. (Otherwise, we could keep using the PeerID in the MinerInfo state.) It's my understanding that message.peerID is the important one, and therefore the message should be signed using message.peerID.

I understand the example showing PeerID verification is outdated now. I am just flagging this as something to consider while updating the README for the new version.

Matching minerID is unnecessary here. We are already matching that in the contract. It is the function input.
I can show certain things in example but no way write the exact code folks might need.

You last comment is very confusing. Please write out in detail what exactly are you expecting here.

FYI:

We sign message using the on chain peerID.

Signing it with IPNI specific peerID is useless. This would not be secure enough. IPNI ads are already signed by this peerID for verification.

So, I am not sure why you think signing with IPNI specific peerID would be better.

bajtos

Thank you for simplifying the contract!

I have a few comments to consider, see above.

The new version looks good to me 👍🏻 Can you please let me know what is the address of the mainnet deployment we can use in Spark and the FRC?

bajtos reviewed Jan 29, 2025

View reviewed changes

LexLuthr force-pushed the feat/spark-contract branch from 37d479e to e174761 Compare January 31, 2025 08:37

LexLuthr added 3 commits January 31, 2025 14:12

spark contract

9283373

add README, use correct key

25872f5

make contract immutable

f4aa99b

LexLuthr force-pushed the feat/spark-contract branch from e174761 to f4aa99b Compare January 31, 2025 08:42

magik6k and others added 2 commits January 31, 2025 10:23

make gen

8720293

fix log fatal

167c13b

magik6k reviewed Jan 31, 2025

View reviewed changes

market/ipni/ipni-provider/spark.go Outdated Show resolved Hide resolved

magik6k reviewed Jan 31, 2025

View reviewed changes

market/ipni/ipni-provider/spark.go Outdated Show resolved Hide resolved

LexLuthr force-pushed the feat/spark-contract branch from fec0bf3 to 652fbac Compare January 31, 2025 10:02

fix ABI strings

0ddc513

LexLuthr force-pushed the feat/spark-contract branch from 652fbac to 0ddc513 Compare January 31, 2025 10:04

LexLuthr added 4 commits January 31, 2025 15:51

serealize params

323a2f5

update contract getPeerData

a8368cc

update getPeerData return

b6f1e49

update getPeerData return struct

dce763f

This was referenced Jan 31, 2025

FRC: Retrieval Checking Requirements filecoin-project/FIPs#1089

Open

Spark DDO support for Curio CheckerNetwork/roadmap#231

Open

bajtos reviewed Jan 31, 2025

View reviewed changes

market/ipni/spark/sol/contracts/MinerPeerIDMapping.sol Show resolved Hide resolved

bajtos reviewed Jan 31, 2025

View reviewed changes

market/ipni/spark/sol/README.md Show resolved Hide resolved

bajtos reviewed Jan 31, 2025

View reviewed changes

market/ipni/spark/sol/README.md Show resolved Hide resolved

bajtos reviewed Jan 31, 2025

View reviewed changes

bajtos approved these changes Jan 31, 2025

View reviewed changes

LexLuthr added 2 commits February 6, 2025 16:22

fix retrun unpacking

33f7c40

update readme

2c8c229

LexLuthr marked this pull request as ready for review February 6, 2025 13:52

magik6k and others added 3 commits February 6, 2025 16:32

some spack contract fixes

c292244

fix address conversion

f82bca0

log not updatin

f9244ab

pre-merge cleanup

83ec071

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: spark contract #377

feat: spark contract #377

LexLuthr commented Jan 21, 2025 •

edited

Loading

bajtos left a comment

bajtos Jan 29, 2025

LexLuthr Jan 31, 2025

LexLuthr Jan 31, 2025

bajtos Jan 31, 2025

LexLuthr Feb 6, 2025

bajtos Jan 31, 2025

LexLuthr Jan 31, 2025

bajtos Feb 4, 2025

LexLuthr Feb 6, 2025

bajtos left a comment

feat: spark contract #377

Are you sure you want to change the base?

feat: spark contract #377

Conversation

LexLuthr commented Jan 21, 2025 • edited Loading

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

LexLuthr commented Jan 21, 2025 •

edited

Loading