From df8f2f53fcc9acf523ded1e74773e298a0009804 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Tue, 17 Sep 2024 12:59:25 -0700 Subject: [PATCH 1/2] Fix skip list starting from protocol 22 --- src/bucket/BucketManagerImpl.cpp | 11 +- src/bucket/test/BucketManagerTests.cpp | 139 +++++++++++++------------ 2 files changed, 84 insertions(+), 66 deletions(-) diff --git a/src/bucket/BucketManagerImpl.cpp b/src/bucket/BucketManagerImpl.cpp index 200e60e557..a4cf84773e 100644 --- a/src/bucket/BucketManagerImpl.cpp +++ b/src/bucket/BucketManagerImpl.cpp @@ -1120,7 +1120,16 @@ BucketManagerImpl::calculateSkipValues(LedgerHeader& currentHeader) } currentHeader.skipList[1] = currentHeader.skipList[0]; } - currentHeader.skipList[0] = currentHeader.bucketListHash; + + if (protocolVersionStartsFrom(currentHeader.ledgerVersion, + ProtocolVersion::V_22)) + { + currentHeader.skipList[0] = currentHeader.previousLedgerHash; + } + else + { + currentHeader.skipList[0] = currentHeader.bucketListHash; + } } } diff --git a/src/bucket/test/BucketManagerTests.cpp b/src/bucket/test/BucketManagerTests.cpp index fd7653205b..fab6e6d601 100644 --- a/src/bucket/test/BucketManagerTests.cpp +++ b/src/bucket/test/BucketManagerTests.cpp @@ -119,71 +119,80 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") Hash h6 = HashUtils::pseudoRandomForTesting(); Hash h7 = HashUtils::pseudoRandomForTesting(); - // up first entry - LedgerHeader header; - header.ledgerSeq = 5; - header.bucketListHash = h1; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h0); - REQUIRE(header.skipList[1] == h0); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_1; - header.bucketListHash = h2; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h2); - REQUIRE(header.skipList[1] == h0); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_1 * 2; - header.bucketListHash = h3; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h3); - REQUIRE(header.skipList[1] == h0); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_1 * 2 + 1; - header.bucketListHash = h2; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h3); - REQUIRE(header.skipList[1] == h0); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_2; - header.bucketListHash = h4; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h4); - REQUIRE(header.skipList[1] == h0); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_2 + SKIP_1; - header.bucketListHash = h5; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h5); - REQUIRE(header.skipList[1] == h4); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_3 + SKIP_2; - header.bucketListHash = h6; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h6); - REQUIRE(header.skipList[1] == h4); - REQUIRE(header.skipList[2] == h0); - REQUIRE(header.skipList[3] == h0); - - header.ledgerSeq = SKIP_3 + SKIP_2 + SKIP_1; - header.bucketListHash = h7; - calculateSkipValues(header); - REQUIRE(header.skipList[0] == h7); - REQUIRE(header.skipList[1] == h6); - REQUIRE(header.skipList[2] == h4); - REQUIRE(header.skipList[3] == h0); + for (size_t i = 21; i <= 22; ++i) + { + // up first entry + LedgerHeader header; + header.ledgerSeq = 5; + header.ledgerVersion = i; + auto& hashFieldToUse = + protocolVersionIsBefore(header.ledgerVersion, + ProtocolVersion::V_22) + ? header.bucketListHash + : header.previousLedgerHash; + + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h0); + REQUIRE(header.skipList[1] == h0); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_1; + hashFieldToUse = h2; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h2); + REQUIRE(header.skipList[1] == h0); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_1 * 2; + hashFieldToUse = h3; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h3); + REQUIRE(header.skipList[1] == h0); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_1 * 2 + 1; + hashFieldToUse = h2; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h3); + REQUIRE(header.skipList[1] == h0); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_2; + hashFieldToUse = h4; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h4); + REQUIRE(header.skipList[1] == h0); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_2 + SKIP_1; + hashFieldToUse = h5; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h5); + REQUIRE(header.skipList[1] == h4); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_3 + SKIP_2; + hashFieldToUse = h6; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h6); + REQUIRE(header.skipList[1] == h4); + REQUIRE(header.skipList[2] == h0); + REQUIRE(header.skipList[3] == h0); + + header.ledgerSeq = SKIP_3 + SKIP_2 + SKIP_1; + hashFieldToUse = h7; + calculateSkipValues(header); + REQUIRE(header.skipList[0] == h7); + REQUIRE(header.skipList[1] == h6); + REQUIRE(header.skipList[2] == h4); + REQUIRE(header.skipList[3] == h0); + } } }; From df916c806c45fc77e1a08ec7ecaaf49ad3960a68 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Wed, 25 Sep 2024 12:32:34 -0700 Subject: [PATCH 2/2] Update 50 bucket to 500 --- src/bucket/BucketManagerImpl.cpp | 14 ++++++++------ src/bucket/BucketManagerImpl.h | 2 ++ src/bucket/test/BucketManagerTests.cpp | 23 +++++++++++++---------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/bucket/BucketManagerImpl.cpp b/src/bucket/BucketManagerImpl.cpp index a4cf84773e..620bda6484 100644 --- a/src/bucket/BucketManagerImpl.cpp +++ b/src/bucket/BucketManagerImpl.cpp @@ -1101,16 +1101,19 @@ BucketManagerImpl::getBloomLookupMeter() const void BucketManagerImpl::calculateSkipValues(LedgerHeader& currentHeader) { + bool startsFromV22 = protocolVersionStartsFrom(currentHeader.ledgerVersion, + ProtocolVersion::V_22); + auto skip1 = startsFromV22 ? SKIP_1_V22 : SKIP_1; - if ((currentHeader.ledgerSeq % SKIP_1) == 0) + if ((currentHeader.ledgerSeq % skip1) == 0) { - int v = currentHeader.ledgerSeq - SKIP_1; + int v = currentHeader.ledgerSeq - skip1; if (v > 0 && (v % SKIP_2) == 0) { - v = currentHeader.ledgerSeq - SKIP_2 - SKIP_1; + v = currentHeader.ledgerSeq - SKIP_2 - skip1; if (v > 0 && (v % SKIP_3) == 0) { - v = currentHeader.ledgerSeq - SKIP_3 - SKIP_2 - SKIP_1; + v = currentHeader.ledgerSeq - SKIP_3 - SKIP_2 - skip1; if (v > 0 && (v % SKIP_4) == 0) { @@ -1121,8 +1124,7 @@ BucketManagerImpl::calculateSkipValues(LedgerHeader& currentHeader) currentHeader.skipList[1] = currentHeader.skipList[0]; } - if (protocolVersionStartsFrom(currentHeader.ledgerVersion, - ProtocolVersion::V_22)) + if (startsFromV22) { currentHeader.skipList[0] = currentHeader.previousLedgerHash; } diff --git a/src/bucket/BucketManagerImpl.h b/src/bucket/BucketManagerImpl.h index 580656c5da..b2d0b41579 100644 --- a/src/bucket/BucketManagerImpl.h +++ b/src/bucket/BucketManagerImpl.h @@ -195,6 +195,8 @@ class BucketManagerImpl : public BucketManager }; #define SKIP_1 50 +#define SKIP_1_V22 500 + #define SKIP_2 5000 #define SKIP_3 50000 #define SKIP_4 500000 diff --git a/src/bucket/test/BucketManagerTests.cpp b/src/bucket/test/BucketManagerTests.cpp index fab6e6d601..544004ffb8 100644 --- a/src/bucket/test/BucketManagerTests.cpp +++ b/src/bucket/test/BucketManagerTests.cpp @@ -125,11 +125,12 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") LedgerHeader header; header.ledgerSeq = 5; header.ledgerVersion = i; - auto& hashFieldToUse = - protocolVersionIsBefore(header.ledgerVersion, - ProtocolVersion::V_22) - ? header.bucketListHash - : header.previousLedgerHash; + + bool isBeforeV22 = protocolVersionIsBefore( + header.ledgerVersion, ProtocolVersion::V_22); + + auto& hashFieldToUse = isBeforeV22 ? header.bucketListHash + : header.previousLedgerHash; calculateSkipValues(header); REQUIRE(header.skipList[0] == h0); @@ -137,7 +138,9 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") REQUIRE(header.skipList[2] == h0); REQUIRE(header.skipList[3] == h0); - header.ledgerSeq = SKIP_1; + auto skip1 = isBeforeV22 ? SKIP_1 : SKIP_1_V22; + + header.ledgerSeq = skip1; hashFieldToUse = h2; calculateSkipValues(header); REQUIRE(header.skipList[0] == h2); @@ -145,7 +148,7 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") REQUIRE(header.skipList[2] == h0); REQUIRE(header.skipList[3] == h0); - header.ledgerSeq = SKIP_1 * 2; + header.ledgerSeq = skip1 * 2; hashFieldToUse = h3; calculateSkipValues(header); REQUIRE(header.skipList[0] == h3); @@ -153,7 +156,7 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") REQUIRE(header.skipList[2] == h0); REQUIRE(header.skipList[3] == h0); - header.ledgerSeq = SKIP_1 * 2 + 1; + header.ledgerSeq = skip1 * 2 + 1; hashFieldToUse = h2; calculateSkipValues(header); REQUIRE(header.skipList[0] == h3); @@ -169,7 +172,7 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") REQUIRE(header.skipList[2] == h0); REQUIRE(header.skipList[3] == h0); - header.ledgerSeq = SKIP_2 + SKIP_1; + header.ledgerSeq = SKIP_2 + skip1; hashFieldToUse = h5; calculateSkipValues(header); REQUIRE(header.skipList[0] == h5); @@ -185,7 +188,7 @@ TEST_CASE("skip list", "[bucket][bucketmanager]") REQUIRE(header.skipList[2] == h0); REQUIRE(header.skipList[3] == h0); - header.ledgerSeq = SKIP_3 + SKIP_2 + SKIP_1; + header.ledgerSeq = SKIP_3 + SKIP_2 + skip1; hashFieldToUse = h7; calculateSkipValues(header); REQUIRE(header.skipList[0] == h7);