Conversation
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | 57c44a7 | Commit Preview URL Branch Preview URL |
Jan 13 2026, 06:53 PM |
| notAfter: bytesToBigInt(data.slice(8, 16)), | ||
| signature: bytesToHex(data.slice(16)), | ||
| } | ||
| const address = await recoverTypedDataAddress({ |
There was a problem hiding this comment.
I looked into why this was async (in principle it should not be). It is only because of this async import:
export async function recoverPublicKey({
hash,
signature,
}: RecoverPublicKeyParameters): Promise<RecoverPublicKeyReturnType> {
const hashHex = isHex(hash) ? hash : toHex(hash)
const { secp256k1 } = await import('@noble/curves/secp256k1')
There was a problem hiding this comment.
oh, that's nasty, but oh well
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Co-authored-by: Jakub Sztandera <oss@kubuxu.com>
| if (hexData.length !== 164) { | ||
| return { | ||
| address: null, | ||
| endorsement: { | ||
| nonce: 0n, | ||
| notAfter: 0n, | ||
| signature: '0x', | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
I want to reduce use of try/catch for expected situations because it can mask unexpected situations
There was a problem hiding this comment.
can you pls move this to examples/cli and make a command for this
There was a problem hiding this comment.
all of those commands in cli are only using calibration. None of them consult env. How do you want those to select the chain?
There was a problem hiding this comment.
@hugomrdias are we blocked on that? should we land this in place and then move it later or do we need to resolve #469 first?
There was a problem hiding this comment.
i just dont like random cli files in the core folder we have the utils folder for that and now the examples/cli for more structured cli/nodejs stuff.
but yes we can land this and fix later
There was a problem hiding this comment.
I don't like that the privateKeyClient/conf pattern stores the private key in a plaintext file. It would be better to use an encrypted keystore for that.
There was a problem hiding this comment.
I added support for foundry keystores: #554
| notAfter: EXPIRY, | ||
| }) | ||
| console.log('Provider:', providerId) | ||
| console.log('Endorsement:', encoded) |
There was a problem hiding this comment.
TODO: curio interface for hex
There was a problem hiding this comment.
|
|
||
| const PRIVATE_KEY = process.env.PRIVATE_KEY | ||
| const ETH_RPC_URL = process.env.ETH_RPC_URL || 'https://api.calibration.node.glif.io/rpc/v1' | ||
| const EXPIRY = process.env.EXPIRY || BigInt(Math.floor(Date.now() / 1000)) + 10368000n |
There was a problem hiding this comment.
comment that this is (now + 4 months)
| } | ||
|
|
||
| const PRIVATE_KEY = process.env.PRIVATE_KEY | ||
| const ETH_RPC_URL = process.env.ETH_RPC_URL || 'https://api.calibration.node.glif.io/rpc/v1' |
There was a problem hiding this comment.
maybe these should be ??
There was a problem hiding this comment.
not strictly necessary but would provide clearer error cases - if you set ETH_RPC_URL to something falsy then it's going to use the default where it probably should error, e.g. "can't connect to RPC provider '0'
| } | ||
| } catch { | ||
| // Skip invalid endorsements | ||
| } |
There was a problem hiding this comment.
I think we can remove this catch now
There was a problem hiding this comment.
Would need to make sure that signatures that fail to recover don't throw. Should have a test case covering this in case that behavior changes.
There was a problem hiding this comment.
👍 looks to me like a failure here would be of the fatal kind now since you're doing so much in decodeEndorsement
| const signature = await signTypedData(client, { | ||
| account: client.account, | ||
| domain: { | ||
| name: 'Storage Endorsement', |
| (results: [ProviderInfo[], ProviderInfo[]], provider: ProviderInfo) => { | ||
| results[ | ||
| preferEndorsements.some( | ||
| (endorsement: Address) => endorsement in (provider.products.PDP?.data.endorsements ?? {}) |
There was a problem hiding this comment.
could be cleaned up by pulling provider.products.PDP?.data.endorsements ?? {} into a local variable inside the arrow func, then we might be able to avoid the multi-line mess that's making this hard to read
|
Overall fine by me, just a few minor suggestions in there and one major concern to be resolved regarding the CLI tool. |
Much of this work can be reused with the Endorsement Registry approach. |
Continuing on branch |
Co-authored by @Kubuxu
Reviewer @rvagg @hugomrdias
Context
For #312, when we are selecting the providers, we prefer that at least one provider has a prime endorsement.
If there aren't any prime endorsements, we do not panic.
We will surface these endorsements as product capabilities in the service provider registry.
Changes
nullon failure instead of throwing an exceptioncreateContextandcreateContextstopreferEndorsementsinsmartSelectProviderif no other providers have been selectedref: FIL-13, FIL-14