core_tests: check service node registration and expiration #195

msgmaxim · 2018-08-30T01:05:44Z

No description provided.

Doy-lee

Note the terminology difference in the code base for expiration and deregistration. Deregistration is the term for getting voted off by other service nodes. Expiring is the term for your stake transpiring past the staking period.

Adding in the additional checks in this test that we're receiving rewards between registration and expiring as mentioned by Jcktm and then this test is looking good for merge.

Doy-lee · 2018-08-30T02:39:49Z

src/cryptonote_core/service_node_list.cpp

@@ -804,7 +804,7 @@ namespace service_nodes

  uint64_t service_node_list::get_staking_requirement_lock_blocks() const


This does not need to be a member function of services node list, anywhere you have access to a service node list you will have access to the service nodes namespace.

Doy-lee · 2018-08-30T02:43:25Z

src/cryptonote_config.h

@@ -52,6 +52,7 @@
 #define STAKING_REQUIREMENT_LOCK_BLOCKS_EXCESS          20
 #define STAKING_REQUIREMENT_LOCK_BLOCKS                 (30*24*30)
 #define STAKING_REQUIREMENT_LOCK_BLOCKS_TESTNET         (30*24*2)
+#define STAKING_REQUIREMENT_LOCK_BLOCKS_FAKENET         30
 #define STAKING_PORTIONS                                UINT64_C(0xfffffffffffffffc)


I think the intention here is if we want to force people to use the API to get the staking requirement lock blocks (and the flexibility to change them for hardforks). We define them as constants in the service_nodes::get_staking_requirements...() and remove the #defines in cryptonote config.

Doy-lee · 2018-08-30T03:17:56Z

tests/core_tests/service_nodes.cpp

-  m_alice_service_node_keys = keypair::generate(hw::get_device("default"));
+  /// NOTE: we don't generate random keys here, because the verification will call its own constructor
+  constexpr char* pub_key_str = "cf6ae1d4e902f7a85af58d6069c29f09702e25fd07cf28d359e64401002db2a1";
+  constexpr char* sec_key_str = "ead4cc692c4237f62f9cefaf5e106995b2dda79a29002a546876f9ee7abcc203";


A nice trick with these is to make these char pub_key_str[] which lets the compiler preserve the size of the string. You can then use sizeof(pub_key_str) to get the length of the string including the null-terminator.

You don't have to change this, just a tidbit.

msgmaxim · 2018-08-31T02:11:03Z

changed get_balance to include service node rewards
added a test to check if the service node rewards have been received by alice
renamed deregistered -> expired

- include service nodes rewards when calculating account's balance - check that service nodes rewards have been received

Doy-lee · 2018-09-03T02:10:05Z

src/daemon/rpc_command_executor.cpp

@@ -2037,7 +2037,7 @@ static void print_service_node_list_state(cryptonote::network_type nettype, uint
    // Print Expiry Info
    {
      uint64_t expiry_height = entry.registration_height;
-      expiry_height += (nettype == cryptonote::TESTNET) ? STAKING_REQUIREMENT_LOCK_BLOCKS_TESTNET : STAKING_REQUIREMENT_LOCK_BLOCKS;
+      expiry_height += get_staking_requirement_lock_blocks(nettype);


There's a couple of calls here that you changed to the get_staking_requirement_lock_blocks that needs the service_nodes namespace otherwise it doesn't compile.

Doy-lee · 2018-09-03T02:10:25Z

src/simplewallet/simplewallet.cpp

@@ -5089,7 +5089,7 @@ bool simple_wallet::stake_main(
  uint64_t fetched_blocks;
  m_wallet->refresh(0, fetched_blocks);

-  uint64_t staking_requirement_lock_blocks = (m_wallet->nettype() == cryptonote::TESTNET ? STAKING_REQUIREMENT_LOCK_BLOCKS_TESTNET : STAKING_REQUIREMENT_LOCK_BLOCKS);
+  uint64_t staking_requirement_lock_blocks = get_staking_requirement_lock_blocks(m_wallet->nettype());


This one as well.

…ants

msgmaxim · 2018-09-04T00:46:15Z

@Doy-lee please review the latest commit

* core: submit uptime proof immediately after registering * Increase visibility of autostaking prompts * quorum_cop: changed uptime proof prune timeout to 2 hours 10 minutes * cleanup: removed scope limiting block * check_tx_inputs: fix deregister double spend test to include deregisters from other heights * config: new testnet network id, genesis tx, and version bump * wallet2: fix testnet wallet blockheight approximation * Fix change in address format in RPC which broke parsing and pooling contributors (#184) * Fix service node endpoints for RPC to also use stdout (#185) * fixed some further rct core tests (#180) * Fix service node state by calling detached hooks on failure to switch to alt chain (#188) * fixed block verification core tests (#186) * fixed block verification core tests * core tests: removed gen_block_miner_tx_out_is_small which is only relevant to hardfork version 1 * Don't consider expired deregistrations when filling block template * Add unit tests for getting staking requirement (#191) * First service node test (#190) * core_tests: added service node tests * core_tests: check balance after registration tx * Fix underflow for popping rollback events (#189) * Move deregistration age check into check_tx_inputs * Zero initialise rct_signatures member txnFee is a uint64_t and has uninit values * Enforce that deregisters must be 0 fee since we skip checks * Add unit tests for vote validation (#193) * Add unit tests for deregistration validation (#194) * Mainnet checkpoint 86535, testnet 3591, 4166 * Bump version number * Add print_sr for getting staking requirement (#198) * Misc bugfixes (#203) * removed unnecessary cast to double during txfee+coinbase calculation * simplewallet: increased autostaking interval from 2 minutes to 40 * Fix casting issues from uint to int (#204) * core_tests: check service node registration and expiration (#195) * core_tests: check service node registration and deregistration * core_tests for service nodes: - include service nodes rewards when calculating account's balance - check that service nodes rewards have been received * fixed namespace error; reduced the scope of staking requirement constants * On blockchain inc/dec mark deregisters relayble based on age (#201) * Service nodes restore only 1 rollback bug (#206) * Fix restore 1 rollback event, ensure prevent rollback is always added * Remove adding prevent_rollback event at init It gets called in on block added generic anyway. * Log db exception, fix relation operators for vote/deregister lifetime (#207) * Filter relayable deregisters w/ check_tx_inputs instead of blockchain callbacks * Bump version to 0.3.7-beta * fix build with GCC 8.1.0 (#211) * Add temp hardfork rule in testnet for deregister lifetimes (#210) * Update testnet, remove testnet forks, remove checkpoints, update blockheight estimate (#212) * Don't ban peers for a bad vote, just drop their connection (#213) * Update to version 0.3.0 release candidate (#215)

* core: submit uptime proof immediately after registering * Increase visibility of autostaking prompts * quorum_cop: changed uptime proof prune timeout to 2 hours 10 minutes * cleanup: removed scope limiting block * check_tx_inputs: fix deregister double spend test to include deregisters from other heights * config: new testnet network id, genesis tx, and version bump * wallet2: fix testnet wallet blockheight approximation * Fix change in address format in RPC which broke parsing and pooling contributors (oxen-io#184) * Fix service node endpoints for RPC to also use stdout (oxen-io#185) * fixed some further rct core tests (oxen-io#180) * Fix service node state by calling detached hooks on failure to switch to alt chain (oxen-io#188) * fixed block verification core tests (oxen-io#186) * fixed block verification core tests * core tests: removed gen_block_miner_tx_out_is_small which is only relevant to hardfork version 1 * Don't consider expired deregistrations when filling block template * Add unit tests for getting staking requirement (oxen-io#191) * First service node test (oxen-io#190) * core_tests: added service node tests * core_tests: check balance after registration tx * Fix underflow for popping rollback events (oxen-io#189) * Move deregistration age check into check_tx_inputs * Zero initialise rct_signatures member txnFee is a uint64_t and has uninit values * Enforce that deregisters must be 0 fee since we skip checks * Add unit tests for vote validation (oxen-io#193) * Add unit tests for deregistration validation (oxen-io#194) * Mainnet checkpoint 86535, testnet 3591, 4166 * Bump version number * Add print_sr for getting staking requirement (oxen-io#198) * Misc bugfixes (oxen-io#203) * removed unnecessary cast to double during txfee+coinbase calculation * simplewallet: increased autostaking interval from 2 minutes to 40 * Fix casting issues from uint to int (oxen-io#204) * core_tests: check service node registration and expiration (oxen-io#195) * core_tests: check service node registration and deregistration * core_tests for service nodes: - include service nodes rewards when calculating account's balance - check that service nodes rewards have been received * fixed namespace error; reduced the scope of staking requirement constants * On blockchain inc/dec mark deregisters relayble based on age (oxen-io#201) * Service nodes restore only 1 rollback bug (oxen-io#206) * Fix restore 1 rollback event, ensure prevent rollback is always added * Remove adding prevent_rollback event at init It gets called in on block added generic anyway. * Log db exception, fix relation operators for vote/deregister lifetime (oxen-io#207) * Filter relayable deregisters w/ check_tx_inputs instead of blockchain callbacks * Bump version to 0.3.7-beta * fix build with GCC 8.1.0 (oxen-io#211) * Add temp hardfork rule in testnet for deregister lifetimes (oxen-io#210) * Update testnet, remove testnet forks, remove checkpoints, update blockheight estimate (oxen-io#212) * Don't ban peers for a bad vote, just drop their connection (oxen-io#213) * Update to version 0.3.0 release candidate (oxen-io#215)

Doy-lee reviewed Aug 30, 2018

View reviewed changes

msgmaxim force-pushed the service-node-tests branch from 5e3471a to c6fe98b Compare August 31, 2018 02:07

msgmaxim changed the title ~~core_tests: check service node registration and deregistration~~ core_tests: check service node registration and expiration Aug 31, 2018

msgmaxim added 2 commits September 3, 2018 17:20

core_tests: check service node registration and deregistration

435f76e

core_tests for service nodes:

dc455b1

- include service nodes rewards when calculating account's balance - check that service nodes rewards have been received

msgmaxim force-pushed the service-node-tests branch 2 times, most recently from 1a0dc28 to dc455b1 Compare September 3, 2018 07:26

Doy-lee reviewed Sep 3, 2018

View reviewed changes

fixed namespace error; reduced the scope of staking requirement const…

749418e

…ants

Doy-lee approved these changes Sep 4, 2018

View reviewed changes

Doy-lee merged commit dfae876 into oxen-io:dev Sep 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core_tests: check service node registration and expiration #195

core_tests: check service node registration and expiration #195

msgmaxim commented Aug 30, 2018

Doy-lee left a comment

Doy-lee Aug 30, 2018

Doy-lee Aug 30, 2018 •

edited

Doy-lee Aug 30, 2018

msgmaxim commented Aug 31, 2018

Doy-lee Sep 3, 2018

Doy-lee Sep 3, 2018

msgmaxim commented Sep 4, 2018

		@@ -804,7 +804,7 @@ namespace service_nodes

		uint64_t service_node_list::get_staking_requirement_lock_blocks() const

core_tests: check service node registration and expiration #195

core_tests: check service node registration and expiration #195

Conversation

msgmaxim commented Aug 30, 2018

Doy-lee left a comment

Choose a reason for hiding this comment

Doy-lee Aug 30, 2018

Choose a reason for hiding this comment

Doy-lee Aug 30, 2018 • edited

Choose a reason for hiding this comment

Doy-lee Aug 30, 2018

Choose a reason for hiding this comment

msgmaxim commented Aug 31, 2018

Doy-lee Sep 3, 2018

Choose a reason for hiding this comment

Doy-lee Sep 3, 2018

Choose a reason for hiding this comment

msgmaxim commented Sep 4, 2018

Doy-lee Aug 30, 2018 •

edited