From 8cf7c9548a06944fc128d88f01d213aec3d1102f Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Fri, 28 Feb 2020 14:48:24 -0800 Subject: [PATCH] Remove conditionals for fix1528 enabled 14Nov2017 --- src/ripple/app/consensus/RCLConsensus.cpp | 6 - src/ripple/consensus/Consensus.h | 5 +- src/ripple/consensus/ConsensusParms.h | 11 -- src/ripple/protocol/Feature.h | 2 +- src/ripple/protocol/impl/Feature.cpp | 2 +- src/test/consensus/Consensus_test.cpp | 230 +++++++++------------- 6 files changed, 93 insertions(+), 163 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index b93cf6347..50143d504 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -119,9 +119,6 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash) // Notify inbound transactions of the new ledger sequence number inboundTransactions_.newRound(built->info().seq); - // Use the ledger timing rules of the acquired ledger - parms_.useRoundedCloseTime = built->rules().enabled(fix1528); - return RCLCxLedger(built); } @@ -913,9 +910,6 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr) // Notify inbound ledgers that we are starting a new round inboundTransactions_.newRound(prevLgr.seq()); - // Use parent ledger's rules to determine whether to use rounded close time - parms_.useRoundedCloseTime = prevLgr.ledger_->rules().enabled(fix1528); - // propose only if we're in sync with the network (and validating) return validating_ && synced; } diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index 2cbf78910..c6c03fea8 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -1697,10 +1697,7 @@ template NetClock::time_point Consensus::asCloseTime(NetClock::time_point raw) const { - if (adaptor_.parms().useRoundedCloseTime) - return roundCloseTime(raw, closeResolution_); - else - return effCloseTime(raw, closeResolution_, previousLedger_.closeTime()); + return roundCloseTime(raw, closeResolution_); } } // namespace ripple diff --git a/src/ripple/consensus/ConsensusParms.h b/src/ripple/consensus/ConsensusParms.h index 6e2d88f92..94e8d3ff4 100644 --- a/src/ripple/consensus/ConsensusParms.h +++ b/src/ripple/consensus/ConsensusParms.h @@ -138,17 +138,6 @@ struct ConsensusParms //! Percentage of nodes required to reach agreement on ledger close time std::size_t avCT_CONSENSUS_PCT = 75; - - //-------------------------------------------------------------------------- - - /** Whether to use roundCloseTime or effCloseTime for reaching close time - consensus. - This was added to migrate from effCloseTime to roundCloseTime on the - live network. The desired behavior (as given by the default value) is - to use roundCloseTime during consensus voting and then use effCloseTime - when accepting the consensus ledger. - */ - bool useRoundedCloseTime = true; }; } // ripple diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index aa67e9613..6f6b6e94f 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -381,7 +381,7 @@ extern uint256 const fix1201; extern uint256 const fix1512; extern uint256 const fix1513; extern uint256 const fix1523; -extern uint256 const fix1528; +extern uint256 const retiredFix1528; extern uint256 const featureDepositAuth; extern uint256 const featureChecks; extern uint256 const fix1571; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 3136144b1..fd36ff254 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -172,7 +172,7 @@ uint256 const fix1201 = *getRegisteredFeature("fix1201"); uint256 const fix1512 = *getRegisteredFeature("fix1512"); uint256 const fix1513 = *getRegisteredFeature("fix1513"); uint256 const fix1523 = *getRegisteredFeature("fix1523"); -uint256 const fix1528 = *getRegisteredFeature("fix1528"); +uint256 const retiredFix1528 = *getRegisteredFeature("fix1528"); uint256 const featureDepositAuth = *getRegisteredFeature("DepositAuth"); uint256 const featureChecks = *getRegisteredFeature("Checks"); uint256 const fix1571 = *getRegisteredFeature("fix1571"); diff --git a/src/test/consensus/Consensus_test.cpp b/src/test/consensus/Consensus_test.cpp index 6075a320d..6dab15685 100644 --- a/src/test/consensus/Consensus_test.cpp +++ b/src/test/consensus/Consensus_test.cpp @@ -592,153 +592,103 @@ public: // This is a specialized test engineered to yield ledgers with different // close times even though the peers believe they had close time // consensus on the ledger. + ConsensusParms parms; - for (bool useRoundedCloseTime : {false, true}) + Sim sim; + + // This requires a group of 4 fast and 2 slow peers to create a + // situation in which a subset of peers requires seeing additional + // proposals to declare consensus. + PeerGroup slow = sim.createGroup(2); + PeerGroup fast = sim.createGroup(4); + PeerGroup network = fast + slow; + + for (Peer* peer : network) + peer->consensusParms = parms; + + // Connected trust graph + network.trust(network); + + // Fast and slow network connections + fast.connect( + fast, date::round(0.2 * parms.ledgerGRANULARITY)); + slow.connect( + network, + date::round(1.1 * parms.ledgerGRANULARITY)); + + // Run to the ledger *prior* to decreasing the resolution + sim.run(increaseLedgerTimeResolutionEvery - 2); + + // In order to create the discrepency, we want a case where if + // X = effCloseTime(closeTime, resolution, parentCloseTime) + // X != effCloseTime(X, resolution, parentCloseTime) + // + // That is, the effective close time is not a fixed point. This can + // happen if X = parentCloseTime + 1, but a subsequent rounding goes + // to the next highest multiple of resolution. + + // So we want to find an offset (now + offset) % 30s = 15 + // (now + offset) % 20s = 15 + // This way, the next ledger will close and round up Due to the + // network delay settings, the round of consensus will take 5s, so + // the next ledger's close time will + + NetClock::duration when = network[0]->now().time_since_epoch(); + + // Check we are before the 30s to 20s transition + NetClock::duration resolution = + network[0]->lastClosedLedger.closeTimeResolution(); + BEAST_EXPECT(resolution == NetClock::duration{30s}); + + while ( + ((when % NetClock::duration{30s}) != NetClock::duration{15s}) || + ((when % NetClock::duration{20s}) != NetClock::duration{15s})) + when += 1s; + // Advance the clock without consensus running (IS THIS WHAT + // PREVENTS IT IN PRACTICE?) + sim.scheduler.step_for( + NetClock::time_point{when} - network[0]->now()); + + // Run one more ledger with 30s resolution + sim.run(1); + if (BEAST_EXPECT(sim.synchronized())) { - ConsensusParms parms; - parms.useRoundedCloseTime = useRoundedCloseTime; - - Sim sim; - - // This requires a group of 4 fast and 2 slow peers to create a - // situation in which a subset of peers requires seeing additional - // proposals to declare consensus. - PeerGroup slow = sim.createGroup(2); - PeerGroup fast = sim.createGroup(4); - PeerGroup network = fast + slow; - + // close time should be ahead of clock time since we engineered + // the close time to round up for (Peer* peer : network) - peer->consensusParms = parms; - - // Connected trust graph - network.trust(network); - - // Fast and slow network connections - fast.connect( - fast, date::round(0.2 * parms.ledgerGRANULARITY)); - slow.connect( - network, - date::round(1.1 * parms.ledgerGRANULARITY)); - - // Run to the ledger *prior* to decreasing the resolution - sim.run(increaseLedgerTimeResolutionEvery - 2); - - // In order to create the discrepency, we want a case where if - // X = effCloseTime(closeTime, resolution, parentCloseTime) - // X != effCloseTime(X, resolution, parentCloseTime) - // - // That is, the effective close time is not a fixed point. This can - // happen if X = parentCloseTime + 1, but a subsequent rounding goes - // to the next highest multiple of resolution. - - // So we want to find an offset (now + offset) % 30s = 15 - // (now + offset) % 20s = 15 - // This way, the next ledger will close and round up Due to the - // network delay settings, the round of consensus will take 5s, so - // the next ledger's close time will - - NetClock::duration when = network[0]->now().time_since_epoch(); - - // Check we are before the 30s to 20s transition - NetClock::duration resolution = - network[0]->lastClosedLedger.closeTimeResolution(); - BEAST_EXPECT(resolution == NetClock::duration{30s}); - - while ( - ((when % NetClock::duration{30s}) != NetClock::duration{15s}) || - ((when % NetClock::duration{20s}) != NetClock::duration{15s})) - when += 1s; - // Advance the clock without consensus running (IS THIS WHAT - // PREVENTS IT IN PRACTICE?) - sim.scheduler.step_for( - NetClock::time_point{when} - network[0]->now()); - - // Run one more ledger with 30s resolution - sim.run(1); - if (BEAST_EXPECT(sim.synchronized())) { - // close time should be ahead of clock time since we engineered - // the close time to round up - for (Peer* peer : network) - { - BEAST_EXPECT( - peer->lastClosedLedger.closeTime() > peer->now()); - BEAST_EXPECT(peer->lastClosedLedger.closeAgree()); - } - } - - // All peers submit their own ID as a transaction - for (Peer* peer : network) - peer->submit(Tx{static_cast(peer->id)}); - - // Run 1 more round, this time it will have a decreased - // resolution of 20 seconds. - - // The network delays are engineered so that the slow peers - // initially have the wrong tx hash, but they see a majority - // of agreement from their peers and declare consensus - // - // The trick is that everyone starts with a raw close time of - // 84681s - // Which has - // effCloseTime(86481s, 20s, 86490s) = 86491s - // However, when the slow peers update their position, they change - // the close time to 86451s. The fast peers declare consensus with - // the 86481s as their position still. - // - // When accepted the ledger - // - fast peers use eff(86481s) -> 86491s as the close time - // - slow peers use eff(eff(86481s)) -> eff(86491s) -> 86500s! - - sim.run(1); - - if (parms.useRoundedCloseTime) - { - BEAST_EXPECT(sim.synchronized()); - } - else - { - // Not currently synchronized - BEAST_EXPECT(!sim.synchronized()); - - // All slow peers agreed on LCL - BEAST_EXPECT(std::all_of( - slow.begin(), slow.end(), [&slow](Peer const* p) { - return p->lastClosedLedger.id() == - slow[0]->lastClosedLedger.id(); - })); - - // All fast peers agreed on LCL - BEAST_EXPECT(std::all_of( - fast.begin(), fast.end(), [&fast](Peer const* p) { - return p->lastClosedLedger.id() == - fast[0]->lastClosedLedger.id(); - })); - - Ledger const& slowLCL = slow[0]->lastClosedLedger; - Ledger const& fastLCL = fast[0]->lastClosedLedger; - - // Agree on parent close and close resolution BEAST_EXPECT( - slowLCL.parentCloseTime() == fastLCL.parentCloseTime()); - BEAST_EXPECT( - slowLCL.closeTimeResolution() == - fastLCL.closeTimeResolution()); - - // Close times disagree ... - BEAST_EXPECT(slowLCL.closeTime() != fastLCL.closeTime()); - // Effective close times agree! The slow peer already rounded! - BEAST_EXPECT( - effCloseTime( - slowLCL.closeTime(), - slowLCL.closeTimeResolution(), - slowLCL.parentCloseTime()) == - effCloseTime( - fastLCL.closeTime(), - fastLCL.closeTimeResolution(), - fastLCL.parentCloseTime())); + peer->lastClosedLedger.closeTime() > peer->now()); + BEAST_EXPECT(peer->lastClosedLedger.closeAgree()); } } + + // All peers submit their own ID as a transaction + for (Peer* peer : network) + peer->submit(Tx{static_cast(peer->id)}); + + // Run 1 more round, this time it will have a decreased + // resolution of 20 seconds. + + // The network delays are engineered so that the slow peers + // initially have the wrong tx hash, but they see a majority + // of agreement from their peers and declare consensus + // + // The trick is that everyone starts with a raw close time of + // 84681s + // Which has + // effCloseTime(86481s, 20s, 86490s) = 86491s + // However, when the slow peers update their position, they change + // the close time to 86451s. The fast peers declare consensus with + // the 86481s as their position still. + // + // When accepted the ledger + // - fast peers use eff(86481s) -> 86491s as the close time + // - slow peers use eff(eff(86481s)) -> eff(86491s) -> 86500s! + + sim.run(1); + + BEAST_EXPECT(sim.synchronized()); } void