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

Add new RPC methods #640

Merged
merged 3 commits into from
Aug 25, 2023
Merged

Add new RPC methods #640

merged 3 commits into from
Aug 25, 2023

Conversation

jsdanielh
Copy link
Member

@jsdanielh jsdanielh commented Aug 4, 2023

  • JsonRpcServer: Add method to get an accounts tree chunk
    • Add RPC method to get an accounts tree chunk: getAccountsTreeChunk. The method receives as string arguments the block hash on which the snapshot will be taken and the startPrefix of the accounts tree chunk. Then it returns an AccountsTreeChunk object.
  • JsonRpcServer: Add new RPC method to get more transaction details
    • Add new RPC method (getTransactionByHash2) that returns more fields than the getTransactionByHash method. The added fields are:
      • toType
      • fromType
      • validityStartHeight
      • networkId
  • Fix some formatting
    • Fix formatting of these files:
      • JsonRpcServer.js
      • TransactionDetails.js

Pull request checklist

  • All tests pass. Demo project builds and runs.
  • I have resolved any merge conflicts.

@jsdanielh jsdanielh changed the title Add RPC method to get an accounts tree chunk, add fields to the transactiond details object and some other minor fixes Add RPC new RPC methods Aug 23, 2023
Fix formatting of these files:
- `JsonRpcServer.js`
- `TransactionDetails.js`
Add new RPC method (`getTransactionByHash2`) that returns more fields
than the `getTransactionByHash` method. The added fields are:
- `toType`
- `fromType`
- `validityStartHeight`
- `networkId`
@@ -103,6 +103,7 @@ class JsonRpcServer {
this._methods.set('getTransactionByBlockHashAndIndex', this.getTransactionByBlockHashAndIndex.bind(this));
this._methods.set('getTransactionByBlockNumberAndIndex', this.getTransactionByBlockNumberAndIndex.bind(this));
this._methods.set('getTransactionByHash', this.getTransactionByHash.bind(this));
this._methods.set('getTransactionByHash2', this.getTransactionByHash2.bind(this));
Copy link
Member

Choose a reason for hiding this comment

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

Why must this be a new method, if you are only adding fields? That is not a breaking change, is it?

Copy link
Member

Choose a reason for hiding this comment

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

It's not a breaking change as long as the client library/code supports it (there are de-serialization libraries that require extra configuration to ignore unknown fields or else they will throw and exception/error out). We can´t change custom client code that works this way (nor is our responsibility), but at least we should check and, if needed, fix this on the community provided libraries, but we didn't want to make this core-js change dependent on that (the idea is to update and release core-js first and then release a new version of the libraries to support the new RPC methods and to make sure they ignore unknown fields, but this second step may take some time).

For PoS we should make sure to release with some kind of versioning scheme that allows us to improve the API while retaining backward compatibility for people who can't update their clients/libraries at the same time as their nodes.

@jeffesquivels jeffesquivels changed the title Add RPC new RPC methods Add new RPC methods Aug 24, 2023
@jeffesquivels jeffesquivels self-assigned this Aug 24, 2023
Add RPC method to get an accounts tree chunk: `getAccountsTreeChunk`.
The method receives as `string` arguments` the block hash on which
the snapshot will be taken and the `startPrefix` of the accounts tree
chunk. Then it returns an `AccountsTreeChunk` object.
@jeffesquivels jeffesquivels merged commit 8a591de into nimiq:master Aug 25, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants