Skip to content

Conversation

@SgtPooki
Copy link
Collaborator

@SgtPooki SgtPooki commented Oct 13, 2025

When calling checkUploadReadiness with a filecoin-pin session key, it should not try to configure allowances.

Note: we need to make this more robust for the future, and should add helpers for consumers and authorization checks of the session key, because some session keys might have varying permissions.

cc @rvagg

related to #101 we likely need to fix logic because if allowances are set we shouldn't need to set them again

@SgtPooki SgtPooki requested a review from rvagg October 13, 2025 21:15
@SgtPooki SgtPooki merged commit 987c4cb into master Oct 13, 2025
12 checks passed
@SgtPooki SgtPooki deleted the fix/using-session-keys branch October 13, 2025 21:18
IPFS_ROOT_CID: 'ipfsRootCid',
},
ADD_PIECES_TYPEHASH: '0x1234567890abcdef',
CREATE_DATA_SET_TYPEHASH: '0xabcdef1234567890',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to remove leftover.. My bad

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rvagg
Copy link
Member

rvagg commented Oct 14, 2025

@SgtPooki what do you think about user feedback in this case? Maybe there's some explicit early-failures we should make in cases we don't want to support, like --auto-fund with a session key. But I'm also concerned about the case where they really do need allowances set up properly and their operation is going to fail because their wallet just isn't ready—we should be able to catch that and fail early with a sensible message rather than having an unhelpful revert show up. Maybe we should extend the return type(s) here to say "something needs to be done but I couldn't do it cause it's a session key"? Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants