Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test target size logic in upgrade test #4111

Merged
merged 4 commits into from
Jan 2, 2024
Merged

Test target size logic in upgrade test #4111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a20c7a
Trigger target size in the sorobanutils upgrade test and increase fee…
sisuresh Dec 22, 2023
ace6d84
Add safety check for settings upgrades
sisuresh Dec 22, 2023
6cdf449
Fix test
sisuresh Dec 22, 2023
249e42f
Update tx meta
sisuresh Dec 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions src/main/SettingsUpgradeUtils.cpp
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we quickly cover this check triggering in some test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

{
// 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
Open in desktop
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 @@ -1615,7 +1614,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 @@ -2739,6 +2737,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 @@ -2843,6 +2871,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 @@ -2918,6 +2968,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,15 @@
"ledger entry size limit enforced" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ],
"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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,15 @@
"ledger entry size limit enforced" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ],
"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