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 blocks_not_found in RPC blocks_info response rather than an error #1950

Merged

Conversation

@guilhermelawless
Copy link
Contributor

commented May 2, 2019

Wiki markup:

INCOMPLETE - needs to be updated with latest changes


Retrieve multiple blocks with additional info

Retrieves a json representations of blocks with block account, transaction amount, block balance, block height in account chain, block local modification timstamp, block confirmation status (since version 19.0), subtype (for state blocks since version 19.0).

Starting from version 19.0, blocks_not_found is an additional field in the response with the blocks that were not found locally. Before this version, an error would occur instead.

Request

{
  "action": "blocks_info",
  "hashes": ["87434F8041869A01C8F6F263B87972D7BA443A72E0A97D7A3FD0CCC2358FD6F9", "0000000000000000000000000000000000000000000000000000000000000001"]
}

Response:

{
  "blocks" : {
    "87434F8041869A01C8F6F263B87972D7BA443A72E0A97D7A3FD0CCC2358FD6F9": {
         "block_account": "xrb_1ipx847tk8o46pwxt5qjdbncjqcbwcc1rrmqnkztrfjy5k7z4imsrata9est",
         "amount": "30000000000000000000000000000000000",
         "balance": "5606157000000000000000000000000000000",
         "height": "58",
         "local_timestamp": "0",
         "confirmed": "false",
       "contents": "{\n
         \"type\": \"state\",\n
         \"account\": \"xrb_1ipx847tk8o46pwxt5qjdbncjqcbwcc1rrmqnkztrfjy5k7z4imsrata9est\",\n
         \"previous\": \"CE898C131AAEE25E05362F247760F8A3ACF34A9796A5AE0D9204E86B0637965E\",\n
         \"representative\": \"xrb_1stofnrxuz3cai7ze75o174bpm7scwj9jn3nxsn8ntzg784jf1gzn1jjdkou\",\n
         \"balance\": \"5606157000000000000000000000000000000\",\n
         \"link\": \"5D1AA8A45F8736519D707FCB375976A7F9AF795091021D7E9C7548D6F45DD8D5\",\n
         \"link_as_account\": \"xrb_1qato4k7z3spc8gq1zyd8xeqfbzsoxwo36a45ozbrxcatut7up8ohyardu1z\",\n
         \"signature\": \"82D41BC16F313E4B2243D14DFFA2FB04679C540C2095FEE7EAE0F2F26880AD56DD48D87A7CC5DD760C5B2D76EE2C205506AA557BF00B60D8DEE312EC7343A501\",\n
         \"work\": \"8a142e07a10996d5\"\n
      }\n",
      "subtype": "send"
     }
  },
  "blocks_not_found": [
    "0000000000000000000000000000000000000000000000000000000000000001"
  ]
}

(...)

@zhyatt zhyatt requested a review from wezrule May 2, 2019

@zhyatt zhyatt added this to the V19.0 milestone May 2, 2019

@zhyatt zhyatt added this to Nice to have in V19 May 2, 2019

@guilhermelawless

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2019

blocks_not_found is always included in the response even if there are no blocks there. Could also not include it if it's empty. I think any of the approaches are reasonable.

@wezrule

This comment has been minimized.

Copy link
Collaborator

commented May 9, 2019

This is a breaking change if someone expects an error if any of the blocks can't be found; this is the pattern we have in quite a few places. There should probably be an optional bool argument to turn this behavior on/off so that it's backwards compatible. What do you think @zhyatt?

@guilhermelawless

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

It's a good point. Could also include the error field if someone is relying on that, along with the rest of fields.

@zhyatt

This comment has been minimized.

Copy link
Collaborator

commented May 9, 2019

I imagine the current behavior is more of an issue than the proposed fix would cause, but we don't want to do breaking changes now. I am not sure including error when there is a partially valid response makes sense, do we use that pattern anywhere else? I would lean toward an optional argument to enable this behavior of providing blocks_not_found in the response, and then when we do a bigger refactor RPC we can evaluate removing that and going to a standard blocks_not_found response because breaking changes for RPC will be happening then anyways.

@guilhermelawless

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2019

@wezrule Do you use the bool to change the type of response anywhere else? Would like to follow the naming convention.

@wezrule

This comment has been minimized.

Copy link
Collaborator

commented May 10, 2019

@wezrule Do you use the bool to change the type of response anywhere else? Would like to follow the naming convention.

Look like raw is an example inside void nano::json_handler::account_history () (json_handler.cpp).

@guilhermelawless guilhermelawless force-pushed the guilhermelawless:rpc/blocks-info-not-found branch from fa32df1 to 68cc6c5 May 10, 2019

Show resolved Hide resolved nano/rpc_test/rpc.cpp Outdated
@wezrule
Copy link
Collaborator

left a comment

Thanks!

@wezrule wezrule merged commit 6559c10 into nanocurrency:master Jun 3, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zhyatt zhyatt moved this from Nice to have to RC 4 (TBD) in V19 Jun 3, 2019

@guilhermelawless guilhermelawless deleted the guilhermelawless:rpc/blocks-info-not-found branch Jun 3, 2019

guilhermelawless added a commit to guilhermelawless/nano-docs that referenced this pull request Jun 3, 2019

zhyatt added a commit to nanocurrency/nano-docs that referenced this pull request Jun 4, 2019

argakiig added a commit that referenced this pull request Jun 11, 2019

Add blocks_not_found in RPC blocks_info response rather than an error (
…#1950)

* Add blocks_not_found in RPC blocks_info response rather than an error

* Lambda capture
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.