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

New print_sn_state_changes command #727

Merged
merged 4 commits into from
Jul 15, 2019
Merged

Conversation

sachaaaaa
Copy link

print_sn_state_changes start_height [end_height]
start_height is mandatory, end_height defaults to the end of the chain.


res.start_height = req.start_height;
res.end_height = end_height;
res.total_deregister = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually don't need to initialise to 0 since the typedef epee::struct_init thingy sets default initialises things it can to 0.

Copy link
Author

Choose a reason for hiding this comment

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

Good to know, thanks! Changed.

}

if (!db.get_blocks(req.start_height, end_height - req.start_height, blocks, txs)) {
MERROR("Could not query block at requested height: " << req.start_height);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of MERROR, fill out the error_resp argument so that it gets returned in the json response.

Copy link
Author

Choose a reason for hiding this comment

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

Done.


if (m_is_rpc)
{
if (!m_rpc_client->json_rpc_request(req, res, "get_sn_state_changes", "Failed to query blockchain checkpoints"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

get_service_node_state_changes so it matches the declaration in core_rpc_server.h and need to update the error message there.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

m_command_lookup.set_handler(
"print_sn_state_changes"
, std::bind(&t_command_parser_executor::print_sn_state_changes, &m_parser, p::_1)
, "print_sn_state_changes start height [end height]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

print_sn_state_changes <start height> [end height]

start height is mandatory so <> notation

Copy link
Author

Choose a reason for hiding this comment

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

Changed.


if (!args_list.empty())
{
std::cout << "use: print_sn_state_changes start height [end height]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here as well

It'd be nice to lookup the usage from the command_handler, but out of scope. Gonna add it to the todo list.

Copy link
Author

Choose a reason for hiding this comment

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

Changed.

end_height = req.end_height;
}

if (!db.get_blocks(req.start_height, end_height - req.start_height, blocks, txs)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should handle the case where req.start_height > end_height

Copy link
Author

Choose a reason for hiding this comment

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

Done.
Also added a + 1 to ensure count is set to 1 when start == end, i.e. when we want to get events for a specific block...

}
if (tx.type == cryptonote::txtype::state_change)
{
const uint8_t hard_fork_version = blocks[i].second.major_version;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is probably what's causing the get_service_node_state_change to fail a lot. The txs and blocks array aren't 1:1. The txs returned from get_blocks looks to be all the transactions spanning across all the blocks requested. So you could call get_transaction_blobs yourself per block so you use the right version for each set of transactions

Copy link
Author

Choose a reason for hiding this comment

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

Ah that probably explains a lot :)
Thanks!

cryptonote::tx_extra_service_node_state_change state_change;
if (!cryptonote::get_service_node_state_change_from_tx_extra(tx.extra, state_change, hard_fork_version))
{
// TODO: This seem to be triggered quite often with hf 11 blocks
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably want this log now, just incase, assuming the fix was correct

Copy link
Author

Choose a reason for hiding this comment

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

Good catch! Uncommented and recompiled, the error logs are not printed anymore.


std::stringstream output;

output << "Service Node state Changes (blocks " << res.start_height << "-" << res.end_height << ")" << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Service Node State Changes

End_height here should be -1 if (res.end_height == sentinel) which by default it gets set to the chain height, so the height is the block you are mining for and not a block that already exists in the blockchain.

Otherwise if it's set, you are grabbing the state changes inclusive of the blocks specified so you don't need the -1.

Copy link
Author

Choose a reason for hiding this comment

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

Changed logic to set the end_height to current_height - 1 when the sentinel is set.

@Doy-lee Doy-lee merged commit 631e3f6 into oxen-io:dev Jul 15, 2019
@sachaaaaa sachaaaaa deleted the print_deregs branch July 15, 2019 03:48
sachaaaaa added a commit to sachaaaaa/loki that referenced this pull request Jul 17, 2019
* new `print_sn_state_changes` command

* Address reviews

* Fix fetching txs for each blocks

* sentinel value is set to current height - 1
Doy-lee pushed a commit to Doy-lee/loki that referenced this pull request Aug 17, 2019
* new `print_sn_state_changes` command

* Address reviews

* Fix fetching txs for each blocks

* sentinel value is set to current height - 1
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.

None yet

2 participants