-
Notifications
You must be signed in to change notification settings - Fork 16
feat: update to latest abi, including SP registry changes #361
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
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | e4fa2ae | Commit Preview URL Branch Preview URL |
Oct 28 2025, 05:34 PM |
|
I've created #362 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM and I can confirm that it works
| } catch { | ||
| return null | ||
| } | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should resolve this before merge
|
im working on this, demo app doesnt work currently |
Signed-off-by: Jakub Sztandera <[email protected]>
9390b86 to
a9bfc88
Compare
| }, | ||
| getProviderWithProduct: async (id: number, productType: number) => { | ||
| if (id === 1 && productType == 0) { | ||
| getPDPService: async (id: number) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is wrong. There is no getPDPService method! Why did you revert my fix that changed it to getProviderWithProduct?
| productType: 0n, | ||
| capabilityKeys: [], | ||
| isActive: false, | ||
| getPDPService: async () => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. There is no getPDPService method! Why did you revert my fix that switched to the new method getProviderWithProduct?
| minProvingPeriodInEpochs: 2880n, | ||
| location: 'US-EAST', | ||
| paymentTokenAddress: '0x0000000000000000000000000000000000000000', | ||
| paymentTokenAddress: '0x0000000000000000000000000000000000000000' as `0x${string}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Hex!
| import { assert } from 'chai' | ||
| import { ethers } from 'ethers' | ||
| import type { Hex } from 'viem' | ||
| import { boolToBytes, bytesToHex, numberToBytes, stringToHex } from 'viem' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bro!
| location: 'EU-WEST', | ||
| paymentTokenAddress: '0x0000000000000000000000000000000000000000', | ||
| } as const | ||
| paymentTokenAddress: '0x0000000000000000000000000000000000000000' as `0x${string}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Hex!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this type assertion is required (as it is for ethers.ZeroAddress), because it's a literal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I tried and it seems it is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another fix is to use as const for the object
| acc[key] = providerWithProduct.productCapabilityValues[i] | ||
| return acc | ||
| }, | ||
| {} as Record<string, `0x${string}`> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Hex!
| // Prepare product data and capabilities | ||
| let productType = 0 // ProductType.PDP | ||
|
|
||
| const [ capabilityKeys, capabilityValues ] = this._convertPDPOfferingToCapabilities(info.pdpOffering, info.capabilities) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bro!
| } | ||
|
|
||
| return capabilities | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bro!
| location: hexToString(capabilities.location), | ||
| paymentTokenAddress: capabilities.paymentTokenAddress, | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bro!
| */ | ||
| private async encodePDPOffering(offering: PDPOffering): Promise<string> { | ||
| const contract = this._getRegistryContract() | ||
| return await contract.encodePDPOffering(offering) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method doesn't exist! why did you bring it back!
This is an awkward mashup of the work @wjmelements is doing in #356 and some ABI update work I was doing as well.
It's likely not perfect, hasn't been properly tested, but should match the latest filecoin-services ABI.
There's some follow-ups in here:
serviceStatus=devin SP capabilities list #358 - I don't thinkdevwill work as-is now, needs to be changed, capabilities need valuescatchs in the registry code, I think this is what @wjmelements was mentioning the other day? And I see in feat(ServiceProviderRegistry): KeyValue productData #356 that he was trying to undo them. I've left them in place here but addedconsole.warn-- if we get a provider and can't decode it then we need to skip it. It could indicate an error in talking to the contract so maybe we need to be more careful with what we ignore and what we allow to propagate. I don't have time to think further about that unfortunately.