Skip to content

Commit

Permalink
Merge pull request #4111 from sisuresh/target
Browse files Browse the repository at this point in the history 
Test target size logic in upgrade test

Reviewed-by: dmkozh
  • Loading branch information
@latobarita
latobarita authored Jan 2, 2024
2 parents 660a981 + 249e42f commit f3ff78d
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 10 deletions.
39 changes: 33 additions & 6 deletions src/main/SettingsUpgradeUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ getUploadTx(PublicKey const& publicKey, SequenceNumber seqNum)

auto& tx = txEnv.v1().tx;
tx.sourceAccount = toMuxedAccount(publicKey);
tx.fee = 10'000'000;
tx.fee = 50'000'000;
tx.seqNum = seqNum;

Preconditions cond;
Expand Down Expand Up @@ -49,7 +49,7 @@ getUploadTx(PublicKey const& publicKey, SequenceNumber seqNum)

tx.ext.v(1);
tx.ext.sorobanData().resources = uploadResources;
tx.ext.sorobanData().resourceFee = 4'000'000;
tx.ext.sorobanData().resourceFee = 35'000'000;

return {txEnv, contractCodeLedgerKey};
}
Expand All @@ -63,7 +63,7 @@ getCreateTx(PublicKey const& publicKey, LedgerKey const& contractCodeLedgerKey,

auto& tx = txEnv.v1().tx;
tx.sourceAccount = toMuxedAccount(publicKey);
tx.fee = 2'000'000;
tx.fee = 5'000'000;
tx.seqNum = seqNum;

Preconditions cond;
Expand Down Expand Up @@ -133,23 +133,50 @@ getCreateTx(PublicKey const& publicKey, LedgerKey const& contractCodeLedgerKey,

tx.ext.v(1);
tx.ext.sorobanData().resources = uploadResources;
tx.ext.sorobanData().resourceFee = 1'000'000;
tx.ext.sorobanData().resourceFee = 3'000'000;

return {txEnv, contractSourceRefLedgerKey, contractID};
}

void
validateConfigUpgradeSet(ConfigUpgradeSet const& upgradeSet)
{
// As of 12/22/2023, this is roughly the size of the buckets. This is used
// in a safety check to prevent us from accidentally reducing the target
// size to a point where upgrade becomes prohibitively expensive. We also
// make sure the growth factor has an upper bound here. This file is just
// used by our command line tooling, so we can update this value whenever we
// want.
static const uint64_t CURRENT_LEDGER_SIZE = 12'500'000'000;
for (auto const& entry : upgradeSet.updatedEntry)
{
if (entry.configSettingID() == CONFIG_SETTING_CONTRACT_LEDGER_COST_V0)
{
if (entry.contractLedgerCost().bucketListTargetSizeBytes <
CURRENT_LEDGER_SIZE * .95 ||
entry.contractLedgerCost().bucketListWriteFeeGrowthFactor >
50'000)
{
throw std::runtime_error("Invalid contractLedgerCost");
}
}
}
}

std::pair<TransactionEnvelope, ConfigUpgradeSetKey>
getInvokeTx(PublicKey const& publicKey, LedgerKey const& contractCodeLedgerKey,
LedgerKey const& contractSourceRefLedgerKey, Hash const& contractID,
ConfigUpgradeSet const& upgradeSet, SequenceNumber seqNum)
{

validateConfigUpgradeSet(upgradeSet);

TransactionEnvelope txEnv;
txEnv.type(ENVELOPE_TYPE_TX);

auto& tx = txEnv.v1().tx;
tx.sourceAccount = toMuxedAccount(publicKey);
tx.fee = 1'000'000;
tx.fee = 100'000'000;
tx.seqNum = seqNum;

Preconditions cond;
Expand Down Expand Up @@ -200,7 +227,7 @@ getInvokeTx(PublicKey const& publicKey, LedgerKey const& contractCodeLedgerKey,

tx.ext.v(1);
tx.ext.sorobanData().resources = invokeResources;
tx.ext.sorobanData().resourceFee = 500'000;
tx.ext.sorobanData().resourceFee = 65'000'000;

ConfigUpgradeSetKey key;
key.contentHash = upgradeHash;
Expand Down
88 changes: 86 additions & 2 deletions src/transactions/test/InvokeHostFunctionTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,6 @@ TEST_CASE("Soroban non-refundable resource fees are stable", "[tx][soroban]")
int64_t expectedNonRefundableFee) {
auto validTx =
makeTx(resources, minInclusionFee, expectedNonRefundableFee);
auto txSize = xdr::xdr_size(validTx->getEnvelope());
auto& app = test.getApp();
// Sanity check the tx fee computation logic.
auto actualFeePair =
Expand Down Expand Up @@ -1620,7 +1619,6 @@ TEST_CASE("contract storage", "[tx][soroban]")
auto appCfg = getTestConfig();
appCfg.ENABLE_SOROBAN_DIAGNOSTIC_EVENTS = true;
SorobanTest test(appCfg, true, modifyCfg);
auto ledgerSeq = test.getLedgerSeq();

auto isSuccess = [](auto resultCode) {
return resultCode == INVOKE_HOST_FUNCTION_SUCCESS;
Expand Down Expand Up @@ -2744,6 +2742,36 @@ TEST_CASE("settings upgrade command line utils", "[tx][soroban][upgrades]")
txsToSign.emplace_back(invokeRes.first);
auto const& upgradeSetKey = invokeRes.second;

// Update the snapshot period and close a ledger to update
// mAverageBucketListSize
app->getLedgerManager()
.getMutableSorobanNetworkConfig()
.setBucketListSnapshotPeriodForTesting(1);
closeLedger(*app);

// Update bucketListTargetSizeBytes so bucketListWriteFeeGrowthFactor comes
// into play
auto const& blSize = app->getLedgerManager()
.getSorobanNetworkConfig()
.getAverageBucketListSize();
{
LedgerTxn ltx(app->getLedgerTxnRoot());
auto costKey = configSettingKey(
ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_V0);

auto costEntry = ltx.load(costKey);
costEntry.current()
.data.configSetting()
.contractLedgerCost()
.bucketListTargetSizeBytes = blSize * .95;
costEntry.current()
.data.configSetting()
.contractLedgerCost()
.bucketListWriteFeeGrowthFactor = 1000;
ltx.commit();
closeLedger(*app);
}

for (auto& txEnv : txsToSign)
{
txEnv.v1().signatures.emplace_back(SignatureUtils::sign(
Expand Down Expand Up @@ -2848,6 +2876,28 @@ TEST_CASE("settings upgrade command line utils", "[tx][soroban][upgrades]")
checkSettings(initialEntries);
}

// The rest are failure tests, so flip back our cost overrides so we can
// check that the initial settings haven't changed.
{
LedgerTxn ltx(app->getLedgerTxnRoot());
auto costKey = configSettingKey(
ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_V0);

auto costEntry = ltx.load(costKey);
costEntry.current()
.data.configSetting()
.contractLedgerCost()
.bucketListTargetSizeBytes =
InitialSorobanNetworkConfig::BUCKET_LIST_TARGET_SIZE_BYTES;
costEntry.current()
.data.configSetting()
.contractLedgerCost()
.bucketListWriteFeeGrowthFactor =
InitialSorobanNetworkConfig::BUCKET_LIST_WRITE_FEE_GROWTH_FACTOR;
ltx.commit();
closeLedger(*app);
}

auto const& lcl = lm.getLastClosedLedgerHeader();

// The only readWrite key in the invoke op is the one that writes the
Expand Down Expand Up @@ -2923,6 +2973,40 @@ TEST_CASE("settings upgrade command line utils", "[tx][soroban][upgrades]")
b.bytes() = upgradeSetBytes;
updateBytes(b);
}

SECTION("Invalid ledger cost parameters")
{
auto costEntryIter =
find_if(upgradeSet.updatedEntry.begin(),
upgradeSet.updatedEntry.end(), [](const auto& entry) {
return entry.configSettingID() ==
CONFIG_SETTING_CONTRACT_LEDGER_COST_V0;
});

SECTION("Invalid bucketListTargetSizeBytes")
{
// 10GB is too low due to the check in validateConfigUpgradeSet
costEntryIter->contractLedgerCost().bucketListTargetSizeBytes =
10'000'000'000;
REQUIRE_THROWS_AS(
getInvokeTx(a1.getPublicKey(), contractCodeLedgerKey,
contractSourceRefLedgerKey, contractID, upgradeSet,
a1.getLastSequenceNumber() + 3),
std::runtime_error);
}

SECTION("Invalid bucketListWriteFeeGrowthFactor")
{
// Value is too high due to the check in validateConfigUpgradeSet
costEntryIter->contractLedgerCost().bucketListWriteFeeGrowthFactor =
50'001;
REQUIRE_THROWS_AS(
getInvokeTx(a1.getPublicKey(), contractCodeLedgerKey,
contractSourceRefLedgerKey, contractID, upgradeSet,
a1.getLastSequenceNumber() + 3),
std::runtime_error);
}
}
}

TEST_CASE("overly large soroban values are handled gracefully", "[soroban]")
Expand Down
10 changes: 9 additions & 1 deletion test-tx-meta-baseline-current/InvokeHostFunctionTests.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,15 @@
],
"refund account merged" : [ "bKDF6V5IzTo=", "y0mODi4DFnM=", "o9mb+GfjtK8=", "HG7OrQT/Ofg=" ],
"settings upgrade" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ],
"settings upgrade command line utils" : [ "TByXElZpITE=", "TByXElZpITE=", "TByXElZpITE=", "TByXElZpITE=" ],
"settings upgrade command line utils" :
[
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE="
],
"state archival" :
[
"bKDF6V5IzTo=",
Expand Down
10 changes: 9 additions & 1 deletion test-tx-meta-baseline-next/InvokeHostFunctionTests.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,15 @@
],
"refund account merged" : [ "bKDF6V5IzTo=", "y0mODi4DFnM=", "o9mb+GfjtK8=", "HG7OrQT/Ofg=" ],
"settings upgrade" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ],
"settings upgrade command line utils" : [ "TByXElZpITE=", "TByXElZpITE=", "TByXElZpITE=", "TByXElZpITE=" ],
"settings upgrade command line utils" :
[
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE=",
"TByXElZpITE="
],
"state archival" :
[
"bKDF6V5IzTo=",
Expand Down

0 comments on commit f3ff78d

Please sign in to comment.