feat(json_rpc): improve and protect getSupplyInfo* methods #2664
Conversation
…supply info was resolved
guidiaz
changed the title
guidiaz
changed the title
guidiaz
requested review from
aesedepece and
drcpu-github
and removed request for
aesedepece
guidiaz
force-pushed
the
json_rpc/improve_get_supply_info_2
branch
from
8d7e117 to
2744cb3
Compare
drcpu-github
left a comment
drcpu-github
left a comment
There was a problem hiding this comment.
I have not actually tested this branch, just added some questions based on a quick read through the code.
| let mut stream = start_client(addr)?; | ||
|
|
||
| let request = r#"{"jsonrpc": "2.0","method": "getSupplyInfo", "id": "1"}"#; | ||
| let request = r#"{"jsonrpc": "2.0","method": "getSupplyInfo2", "id": "1"}"#; |
There was a problem hiding this comment.
Any reason for completely replacing getSupplyInfo with getSupplyInfo2 in the CLI (instead of keeping both as functions)? I think it's kind of confusing the get_supply_info function here aliases to getSupplyInfo2 instead of both being present.
There was a problem hiding this comment.
Not really. Both getSupplyInfo and getSupplyInfo2 commands will be supported in the CLI. However the getSupplyInfo will return non-sensitive fields in the new SupplyInfo struct from now on.
| let mut stream = start_client(addr)?; | ||
|
|
||
| let request = r#"{"jsonrpc": "2.0","method": "getSupplyInfo", "id": "1"}"#; | ||
| let request = r#"{"jsonrpc": "2.0","method": "getSupplyInfo2", "id": "1"}"#; |
There was a problem hiding this comment.
Any reason for replacing getSupplyInfo with getSupplyInfo instead of keeping both as separate functions? I think it's kind of confusing that get_supply_info aliases internally to getSupplyInfo2. I suppose getSupplyInfo is still accessible through the raw interface?
There was a problem hiding this comment.
Indeed. There will be separated functions for each case.
| initial_supply: u64, | ||
| /// Populate RAD hashes index | ||
| rad_hashes_index: bool, | ||
| /// Epoch when getSupplyInfo2 was last resolved. |
There was a problem hiding this comment.
I suppose these variables are not saved to storage (and we do not need storage migrations)? That does imply whenever a node restarts (after a manual intervention or crash), this call will be slow again for the first time?
| } else { | ||
| blocks_missing += 1; | ||
| blocks_missing_reward += block_reward; | ||
| blocks_minted_reward += if wit2_activated && epoch >= wit2_activation_epoch { |
There was a problem hiding this comment.
Is there a point to this double check?
There was a problem hiding this comment.
The first time the blocks_minted_reward is computed after a node restart, self.last_supply_info_epoch will certainly be less than wit2_activation_epoch. However, I guess the loop can be optimized depending on whether self.last_supply_info_epoch is already greater than wit2_activation_epoch, or not.
2 participants
No description provided.