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

Methods generateBatchProof and generateProof return the wrong type #5484

Closed
3 of 10 tasks
en opened this issue Feb 15, 2023 · 3 comments
Closed
3 of 10 tasks

Methods generateBatchProof and generateProof return the wrong type #5484

en opened this issue Feb 15, 2023 · 3 comments

Comments

@en
Copy link

en commented Feb 15, 2023

  • I'm submitting a ...
  • Bug report
  • Feature request
  • Support request
  • Other
  • What is the current behavior and expected behavior?

generateBatchProof: AugmentedRpc<(leafIndices: Vec<u64> | (u64 | AnyNumber | Uint8Array)[], at?: BlockHash | string | Uint8Array) => Observable<MmrLeafProof>>;
/**
* Generate MMR proof for given leaf index.
**/
generateProof: AugmentedRpc<(leafIndex: u64 | AnyNumber | Uint8Array, at?: BlockHash | string | Uint8Array) => Observable<MmrLeafBatchProof>>;

generateBatchProof and generateProof incorrectly use each other's return values.

  • What is the motivation for changing the behavior?

Swap the type of the return value.

  • Please tell us about your environment:
  • Version: master

  • Environment:

    • Node.js
    • Browser
    • Other (limited support for other environments)
  • Language:

    • JavaScript
    • TypeScript (include tsc --version)
    • Other
@jacogr
Copy link
Member

jacogr commented Feb 15, 2023

The issue is that RPCs are completely unversioned.

So in these cases your best approach is actually to submit a PR. (These is actually one open to align with latest Substrate master, see #5479)

In retrospect, the mmr RPCs should not have been added at all - since it changes the whole time at this point and seems very unstable. Changing return types would mean that it would be broken on some older versions and working on newer versions - which is the reverse of what we have.

So while we have "adjust-to-latest PRs", the best approach is actually just a complete removal until these are stabilized instead of playing whack-a-mole with unversioned changing RPCs.

@jacogr
Copy link
Member

jacogr commented Feb 15, 2023

This is actually a duplicate as well, see #5478

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants