Skip to content
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

fix(contracts-sdk): claimAndMint(...) missing stakingContractAddress … #765

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Ansonhkg
Copy link
Collaborator

@Ansonhkg Ansonhkg commented Jan 15, 2025

WHAT

@zach-is-my-name : "

Description of the bug/issue

When I call claimAndMint(...) via the @lit-protocol/contracts-sdk, the SDK only passes 3 parameters (keyType, derivedKeyId, and signatures). However, on the deployed PKPNFT diamond facet, claimAndMint(...) requires a 4th parameter for the stakingContractAddress.

This mismatch leads to either:

Too many arguments errors if my facet expects only 3 parameters but the diamond has 4-arg logic in another place, or
Not enough arguments errors if the diamond facet truly wants all 4 (with stakingContractAddress) but the SDK only calls 3.

Expected Behavior
The SDK should allow me to supply an optional stakingContractAddress (as the 4th parameter) when calling claimAndMint.
The function signature in the SDK’s PKPNFT.sol/PKPNFT.d.ts (or relevant code) should match the on-chain diamond facet signature with all 4 parameters.
Actual Behavior
The SDK only exposes claimAndMint(...) with 3 parameters.
The on-chain PKPNFT facet has a claimAndMint(...) with 4 parameters, causing transaction failure or an ABI mismatch.

Proposed Solution
Update claimAndMint(...) calls in contracts-sdk.js (especially in pkpNftContractUtils.write.claimAndMint) to accept an optional fourth parameter for stakingContractAddress.
If it is not required for some networks, it could default to a zero address or a known address, but it should at least be an optional argument in the method signature.

Additional Context
Observed FunctionNotFound(bytes4 _functionSelector) or too many arguments revert when calling with or without a 4th param.
The snippet from contracts-sdk.js that calls claimAndMint shows [2, derivedKeyId, signatures] only.

Severity of the bug

Can't use the contracts-sdk"

@Ansonhkg Ansonhkg added the 🐞 Bug Fix Something isn't working label Jan 15, 2025
Copy link
Contributor

@MaximusHaximus MaximusHaximus left a comment

Choose a reason for hiding this comment

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

My understanding is that the stakingContractAddress should never have to be user provided -- Was there a bug introduced with contract changes we deployed for cloneNets? It feels broken for us to be expecting someone to provide an address given that they wouldn't know what the value should be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug Fix Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants