AntelopeIO / spring

C++ implementation of the Antelope protocol with Savanna consensus
Other
5 stars 2 forks source link

Producer schedule change doesn't seem correct in Savanna mode #256

Closed greg7mdp closed 3 months ago

greg7mdp commented 3 months ago

Say you have an active schedule with 3 proposers: {"dan"_n,"sam"_n,"pam"_n}

On sam's last block, you change the schedule to: {"dan"_n,"sam"_n,"pam"_n,"cam"_n}

The next 12 blocks are produced by pam as expected.

And then the next block is produced by sam??? I would have expected cam, or dan if the transition hadn't taken place.

Test case reproducing the issue, which fails on the last BOOST_REQUIRE.

#include "fork_test_utilities.hpp"

BOOST_AUTO_TEST_CASE( switch_producers_test_exact ) try {
   tester c;

   c.create_accounts( {"dan"_n,"sam"_n,"pam"_n} );
   c.set_producers( {"dan"_n,"sam"_n,"pam"_n} );

   BOOST_REQUIRE( produce_until_transition( c, "dan"_n, "sam"_n ) );
   c.produce_blocks(35);

   signed_block_ptr b = c.produce_block();
   BOOST_REQUIRE_EQUAL( b->producer.to_string(), "dan"_n.to_string() );

   b = c.produce_block();
   BOOST_REQUIRE_EQUAL( b->producer.to_string(), "sam"_n.to_string() );
   c.produce_blocks(10);
   c.create_accounts( {"cam"_n} );
   c.set_producers( {"dan"_n,"sam"_n,"pam"_n,"cam"_n} );
   c.produce_block();
   BOOST_REQUIRE_EQUAL( b->producer.to_string(), "sam"_n.to_string() );

   // The next 12 blocks should be produced by `pam`.
   for (size_t i=0; i<12; ++i) {
      b = c.produce_block(); // pam produces 12 blocks
      BOOST_REQUIRE_EQUAL( b->producer.to_string(), "pam"_n.to_string() );
   }

   // after `pam`, we should have either `dan` or `cam` (I think `cam`), but we get `sam`???
   b = c.produce_block();
   BOOST_REQUIRE_EQUAL( b->producer.to_string(), "sam"_n.to_string() ); // should fail I think!
   BOOST_REQUIRE( b->producer == "dam"_n || b->producer == "cam"_n);

} FC_LOG_AND_RETHROW()
heifner commented 3 months ago

This is because you have changed the size of the producers. If you used the same number or producers when updating you would see more what you expect. The schedule is a modulus of the producers, so changing the size creates a different index into the schedule according to the time slot.

See. https://github.com/AntelopeIO/spring/blob/f815a29718572a36d8e988988a2c6aa4bb61bd11/libraries/chain/include/eosio/chain/block_header_state_utils.hpp#L23-L27

greg7mdp commented 3 months ago

That explains it, thanks Kevin!

greg7mdp commented 3 months ago

Not an issue