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

daemon-rpc: incorrect doc fixes #2337

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hinto-janai
Copy link
Contributor

@hinto-janai hinto-janai commented Aug 6, 2024

What

Fixes incorrect documentation for daemon RPC calls; requires review into monerod.

Fixes

Table of fixes in this PR, originally tracked here: Cuprate/cuprate#159.

Route/endpoint Link Notes
get_miner_data get_miner_data, rpc/core_rpc_server_commands_defs.h @ 1012 difficulty field is noted as unsigned int, although it is actually a JSON string containing an unsigned int in hex form. Other documentation describes these hex int strings as string.
calc_pow calc_pow, cryptonote_basic/blobdatatype.h @ 39 Type used to describe block_data is blobdata. This is a monerod-specific type alias to std::string. Documentation should explain that it is a hex-encoded string of a block.
/get_outs.bin get_outs.bin, rpc/core_rpc_server_commands_defs.h @ 538..544 Documentation mentions non-existent field: amount
/send_raw_transaction send_raw_transaction, rpc/core_rpc_server_commands_defs.h @ 629..663 Documentation mentions non-existent field: not_rct
/mining_status mining_status, core_rpc_server_commands_defs.h @ 857..873 Documentation notates numbers as int instead of unsigned int. These are unsigned int both in code and elsewhere in documentation.
/get_transaction_pool get_transaction_pool, rpc/core_rpc_server_commands_defs.h @ 1524 Missing documentation for weight field, should be unsigned int; <DESCRIPTION>
flush_txpool flush_txpool Empty "" transaction in example causes error
get_block get_block Documentation states for tx_hashes field: "If there are no other transactions, this will be an empty list." This is not true, monerod's serializer will omit fields completely if the container is empty. The tx_hashes in the json field will exist, however.

Simple proof for get_block:

curl \
    http://127.0.0.1:18081/json_rpc \
    -d '{"jsonrpc":"2.0","id":"0","method":"get_block","params":{"height":0}}' \
    -H 'Content-Type: application/json'

Copy link

netlify bot commented Aug 6, 2024

Deploy Preview for barolo-time-757cf9 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 80274d1
🔍 Latest deploy log https://app.netlify.com/sites/barolo-time-757cf9/deploys/66b174280b4eba0008249a48
😎 Deploy Preview https://deploy-preview-2337--barolo-time-757cf9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hinto-janai hinto-janai marked this pull request as ready for review August 8, 2024 23:56
@@ -604,7 +604,7 @@ Outputs:
* *view_tag* - The 1st byte of a shared secret (used for reducing synchronization time)
* *extra* - Usually called the "transaction ID" but can be used to include any random 32 byte/64 character hex string.
* *rct_signatures* - Contain signatures of tx signers. Coinbased txs do not have signatures.
* *tx_hashes* - List of hashes of non-coinbase transactions in the block. If there are no other transactions, this will be an empty list.
* *tx_hashes* - List of hashes of non-coinbase transactions in the block. If there are no transactions, this field not not be present.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* *tx_hashes* - List of hashes of non-coinbase transactions in the block. If there are no transactions, this field not not be present.
* *tx_hashes* - List of hashes of non-coinbase transactions in the block. If there are no transactions, this field will not be present.

@@ -1887,7 +1887,6 @@ Inputs:
Outputs:

* *outs* - array of structure *outkey* as follows:
* *amount* - unsigned int;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this here for backward compatibility? (from when amounts were public)

Copy link
Collaborator

Choose a reason for hiding this comment

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

iirc we have a few instances of supporting backward compatibility for devs experimenting at different points of the blockchain

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean: it shouldn't be removed if its still needed for older parts of the bc

Copy link
Collaborator

@plowsof plowsof Aug 15, 2024

Choose a reason for hiding this comment

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

* *bg_min_idle_seconds* - unsigned int; Minimum lookback interval in seconds for determining whether the device is idle or not.
* *bg_target* - unsigned int; Maximum percentage cpu use by miner.
* *block_reward* - unsigned int; Base block reward for the next block to be mined.
* *block_target* - unsigned int; The target number of seconds between blocks.
* *difficulty* - unsigned int; Network difficulty (analogous to the strength of the network)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* *difficulty* - unsigned int; Network difficulty (analogous to the strength of the network)
* *difficulty* - unsigned int; Network difficulty (analogous to the strength of the network).

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