Skip to content

Commit

Permalink
Fix bug related to passing in the cost types array in v20 with less t…
Browse files Browse the repository at this point in the history
…han the total cost types
  • Loading branch information
sisuresh committed Apr 8, 2024
1 parent 8f40dc8 commit 6c5d40f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 18 deletions.
25 changes: 23 additions & 2 deletions src/transactions/InvokeHostFunctionOpFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,29 @@ getLedgerInfo(AbstractLedgerTxn& ltx, Application& app,
info.min_temp_entry_ttl =
sorobanConfig.stateArchivalSettings().minTemporaryTTL;
info.max_entry_ttl = sorobanConfig.stateArchivalSettings().maxEntryTTL;
info.cpu_cost_params = toCxxBuf(sorobanConfig.cpuCostParams());
info.mem_cost_params = toCxxBuf(sorobanConfig.memCostParams());

auto cpu = sorobanConfig.cpuCostParams();
auto mem = sorobanConfig.memCostParams();

// The host expects this vector to be the size of the # of cost types in the
// xdr, but the v20 ledgers only have a subset due to the xdr for v21, so we
// need to resize them before passing them to a host that supports v21. This is
// not true for the prev build though because that one will be on the old xdr,
// which is why we compile this resizing out here if we're NOT using prev.
#ifndef ENABLE_PROTOCOL_UPGRADE_VIA_SOROBAN_ENV_HOST_PREV
if (protocolVersionEquals(ltx.loadHeader().current().ledgerVersion,
ProtocolVersion::V_20))
{
auto numCostTypes =
xdr::xdr_traits<ContractCostType>::enum_values().size();
cpu.resize(numCostTypes);
mem.resize(numCostTypes);
}
#endif

info.cpu_cost_params = toCxxBuf(cpu);
info.mem_cost_params = toCxxBuf(mem);

auto& networkID = app.getNetworkID();
info.network_id.reserve(networkID.size());
for (auto c : networkID)
Expand Down
22 changes: 13 additions & 9 deletions src/transactions/test/InvokeHostFunctionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,8 @@ TEST_CASE("refund account merged", "[tx][soroban][merge]")
auto b1 = test.getRoot().create("B", startingBalance);
auto c1 = test.getRoot().create("C", startingBalance);
auto wasm = rust_bridge::get_test_wasm_add_i32();
auto resources = defaultUploadWasmResourcesWithoutFootprint(wasm);
auto resources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(test.getApp()));
auto tx = makeSorobanWasmUploadTx(test.getApp(), a1, wasm, resources, 1000);

auto mergeOp = accountMerge(b1);
Expand All @@ -1156,7 +1157,8 @@ TEST_CASE("refund still happens on bad auth", "[tx][soroban]")
auto a1 = test.getRoot().create("A", startingBalance);
auto b1 = test.getRoot().create("B", startingBalance);
auto wasm = rust_bridge::get_test_wasm_add_i32();
auto resources = defaultUploadWasmResourcesWithoutFootprint(wasm);
auto resources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(test.getApp()));
auto tx = makeSorobanWasmUploadTx(test.getApp(), a1, wasm, resources, 100);

auto a1PreTxBalance = a1.getBalance();
Expand Down Expand Up @@ -1191,7 +1193,8 @@ TEST_CASE("refund test with closeLedger", "[tx][soroban][feebump]")
auto a1StartingBalance = a1.getBalance();

auto wasm = rust_bridge::get_test_wasm_add_i32();
auto resources = defaultUploadWasmResourcesWithoutFootprint(wasm);
auto resources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(test.getApp()));
auto tx = makeSorobanWasmUploadTx(test.getApp(), a1, wasm, resources, 100);

auto r = closeLedger(test.getApp(), {tx});
Expand Down Expand Up @@ -1226,7 +1229,8 @@ TEST_CASE_VERSIONS("refund is sent to fee-bump source",
auto feeBumperStartingBalance = feeBumper.getBalance();

auto wasm = rust_bridge::get_test_wasm_add_i32();
auto resources = defaultUploadWasmResourcesWithoutFootprint(wasm);
auto resources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(test.getApp()));
auto tx =
makeSorobanWasmUploadTx(test.getApp(), a1, wasm, resources, 100);

Expand All @@ -1247,12 +1251,11 @@ TEST_CASE_VERSIONS("refund is sent to fee-bump source",
auto r = closeLedger(test.getApp(), {feeBumpTxFrame});
checkTx(0, r, txFEE_BUMP_INNER_SUCCESS);

bool refundsInFeeCharged = protocolVersionStartsFrom(
bool afterV20 = protocolVersionStartsFrom(
getLclProtocolVersion(test.getApp()), ProtocolVersion::V_21);

auto const txFeeWithRefund = 82'853;
auto const feeCharged =
refundsInFeeCharged ? txFeeWithRefund : 1'040'971;
auto const txFeeWithRefund = afterV20 ? 82'853 : 59'444;
auto const feeCharged = afterV20 ? txFeeWithRefund : 1'040'971;

REQUIRE(
r.at(0).first.result.result.innerResultPair().result.feeCharged ==
Expand Down Expand Up @@ -1290,7 +1293,8 @@ TEST_CASE("buying liabilities plus refund is greater than INT64_MAX",

auto a1PreBalance = a1.getBalance();
auto wasm = rust_bridge::get_test_wasm_add_i32();
auto resources = defaultUploadWasmResourcesWithoutFootprint(wasm);
auto resources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(test.getApp()));
auto tx =
makeSorobanWasmUploadTx(test.getApp(), a1, wasm, resources, 300'000);

Expand Down
28 changes: 22 additions & 6 deletions src/transactions/test/SorobanTxTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,28 @@ makeSorobanWasmUploadTx(Application& app, TestAccount& source,
}

SorobanResources
defaultUploadWasmResourcesWithoutFootprint(RustBuf const& wasm)
defaultUploadWasmResourcesWithoutFootprint(RustBuf const& wasm,
uint32_t ledgerVersion)
{
SorobanResources resources;
resources.instructions =
static_cast<uint32_t>(500'000 + (wasm.data.size() * 5000));
resources.readBytes = 1000;
resources.writeBytes = static_cast<uint32_t>(wasm.data.size() + 150);

// Use a different default starting from v21 for the vm module cache
// changes. Keep the v20 resources the same so we can validate that we're
// not changing fees for the existing protocol.
if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_21))
{
resources.instructions =
static_cast<uint32_t>(500'000 + (wasm.data.size() * 5000));
resources.readBytes = 1000;
resources.writeBytes = static_cast<uint32_t>(wasm.data.size() + 150);
}
else
{
resources.instructions =
static_cast<uint32_t>(500'000 + (wasm.data.size() * 1000));
resources.readBytes = 1000;
resources.writeBytes = static_cast<uint32_t>(wasm.data.size() + 100);
}
return resources;
}

Expand Down Expand Up @@ -859,7 +874,8 @@ SorobanTest::deployWasmContract(RustBuf const& wasm,
{
if (!uploadResources)
{
uploadResources = defaultUploadWasmResourcesWithoutFootprint(wasm);
uploadResources = defaultUploadWasmResourcesWithoutFootprint(
wasm, getLclProtocolVersion(getApp()));
}

if (!createResources)
Expand Down
3 changes: 2 additions & 1 deletion src/transactions/test/SorobanTxTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ ContractExecutable makeWasmExecutable(Hash const& wasmHash);
ContractExecutable makeAssetExecutable(Asset const& asset);

SorobanResources
defaultUploadWasmResourcesWithoutFootprint(RustBuf const& wasm);
defaultUploadWasmResourcesWithoutFootprint(RustBuf const& wasm,
uint32_t ledgerVersion);

// Creates a valid transaction for uploading provided Wasm.
// Fills in the valid footprint automatically in case if `uploadResources`
Expand Down

0 comments on commit 6c5d40f

Please sign in to comment.