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.

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

Refundable fee updates #3886

Merged
merged 2 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
312 changes: 112 additions & 200 deletions Cargo.lock

Large diffs are not rendered by default.

14 changes: 4 additions & 10 deletions src/herder/test/HerderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,6 @@ TEST_CASE("surge pricing", "[herder][txset]")
resources.instructions = 800'000;
resources.readBytes = conf.txMaxReadBytes();
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;
auto sorobanTx =
createUploadWasmTx(*app, acc2, baseFee,
/* refundableFee */ 1200, resources);
Expand All @@ -1665,8 +1664,6 @@ TEST_CASE("surge pricing", "[herder][txset]")
rand_uniform<uint32_t>(1, conf.txMaxReadBytes());
res.writeBytes =
rand_uniform<uint32_t>(1, conf.txMaxWriteBytes());
res.contractEventsSizeBytes = rand_uniform<uint32_t>(
1, conf.txMaxContractEventsSizeBytes());
auto read =
rand_uniform<uint32_t>(0, conf.txMaxReadLedgerEntries());
auto write = rand_uniform<uint32_t>(
Expand All @@ -1690,11 +1687,10 @@ TEST_CASE("surge pricing", "[herder][txset]")
res));
CLOG_INFO(Herder,
"Generated tx with {} instructions, {} read "
"bytes, {} write bytes, {} extended meta "
"data bytes, {} read ledger entries, {} "
"write ledger entries",
res.instructions, res.readBytes, res.writeBytes,
res.contractEventsSizeBytes, read, write);
"bytes, {} write bytes, data bytes, {} read "
"ledger entries, {} write ledger entries",
res.instructions, res.readBytes, res.writeBytes, read,
write);
}
return txs;
};
Expand Down Expand Up @@ -1788,7 +1784,6 @@ TEST_CASE("surge pricing", "[herder][txset]")
resources.instructions = 1;
resources.readBytes = 1;
resources.writeBytes = 1;
resources.contractEventsSizeBytes = 1;

auto smallSorobanLowFee = createUploadWasmTx(
*app, acc4, baseFee / 10, /* refundableFee */ 1200, resources);
Expand Down Expand Up @@ -4519,7 +4514,6 @@ TEST_CASE("do not flood too many soroban transactions",
resources.instructions = 800'000;
resources.readBytes = 2000;
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;

auto genTx = [&](TestAccount& source, bool highFee) {
auto txFee = baseFee;
Expand Down
1 change: 0 additions & 1 deletion src/herder/test/TransactionQueueTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,6 @@ TEST_CASE("Soroban TransactionQueue limits",
resources.instructions = 2'000'000;
resources.readBytes = 2000;
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;

int refundableFee = 1200;
int initialFee = 10'000'000;
Expand Down
3 changes: 0 additions & 3 deletions src/herder/test/TxSetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
resources.instructions = 800'000;
resources.readBytes = 1000;
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;
txs.emplace_back(createUploadWasmTx(*app, source, fee,
/* refundableFee */ 1200,
resources));
Expand Down Expand Up @@ -775,7 +774,6 @@ TEST_CASE("generalized tx set with multiple txs per source account", "[txset]")
resources.instructions = 800'000;
resources.readBytes = 1000;
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;
uint32_t inclusionFee = 500;
uint32_t refundableFee = 10'000;
auto sorobanTx = createUploadWasmTx(*app, root, inclusionFee,
Expand Down Expand Up @@ -824,7 +822,6 @@ TEST_CASE("generalized tx set fees", "[txset]")
resources.instructions = 800'000;
resources.readBytes = 1000;
resources.writeBytes = 1000;
resources.contractEventsSizeBytes = 0;
auto tx = createUploadWasmTx(*app, source, inclusionFee,
refundableFee, resources);
setValidTotalFee(tx, inclusionFee, refundableFee, *app, source);
Expand Down
4 changes: 3 additions & 1 deletion src/ledger/NetworkConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,9 @@ SorobanNetworkConfig::isValidConfigSettingEntry(ConfigSettingEntry const& cfg)
cfg.contractLedgerCost().bucketListWriteFeeGrowthFactor >= 0;
break;
case ConfigSettingID::CONFIG_SETTING_CONTRACT_EVENTS_V0:
valid = cfg.contractEvents().txMaxContractEventsSizeBytes >= 0 &&
valid = cfg.contractEvents().txMaxContractEventsSizeBytes >=
MinimumSorobanNetworkConfig::
TX_MAX_CONTRACT_EVENTS_SIZE_BYTES &&
cfg.contractEvents().feeContractEvents1KB >= 0;
break;
case ConfigSettingID::CONFIG_SETTING_STATE_EXPIRATION:
Expand Down
5 changes: 4 additions & 1 deletion src/ledger/NetworkConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ struct MinimumSorobanNetworkConfig

static constexpr uint32_t MINIMUM_PERSISTENT_ENTRY_LIFETIME = 4'096;
static constexpr uint32_t MAXIMUM_ENTRY_LIFETIME = 535'680; // 31 days

static constexpr uint32_t TX_MAX_CONTRACT_EVENTS_SIZE_BYTES = 200;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the minimum not 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because return value is also counted towards events size and upload wasm and create contract ops return the identifiers. I also added a bit more to be able to get at least some basic events working. I guess minimum could be a bit lower (like 40 bytes), but for initial we'd need a bit more to check that events work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah that's right. Maybe we should change the name to reflect both events and results? Either way this doesn't need to block the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it would require an XDR change and honestly I'm not sure what would be a better name (contractEventsAndReturnValue sounds way too clunky IMO)

};

// Defines the initial values of the network configuration
Expand Down Expand Up @@ -105,7 +107,8 @@ struct InitialSorobanNetworkConfig
static constexpr int64_t FEE_TRANSACTION_SIZE_1KB = 2'000;

// Contract events settings
static constexpr uint32_t TX_MAX_CONTRACT_EVENTS_SIZE_BYTES = 1'000;
static constexpr uint32_t TX_MAX_CONTRACT_EVENTS_SIZE_BYTES =
MinimumSorobanNetworkConfig::TX_MAX_CONTRACT_EVENTS_SIZE_BYTES;
static constexpr int64_t FEE_CONTRACT_EVENTS_SIZE_1KB = 200;

// State expiration settings
Expand Down
2 changes: 1 addition & 1 deletion src/protocol-next/xdr
Submodule xdr updated 1 files
+9 −7 Stellar-transaction.x
6 changes: 3 additions & 3 deletions src/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ rustc-simple-version = "0.1.0"
version = "0.0.17"
git = "https://github.com/stellar/rs-soroban-env"
package = "soroban-env-host"
rev = "048be90e10dfda6486141f96ea86e32fb91681f4"
rev = "dd3d39571e9fdae9af34f7f46243d0794169b411"

# This copy of the soroban host is _optional_ and only enabled during protocol
# transitions. When transitioning from protocol N to N+1, the `curr` copy
Expand All @@ -52,11 +52,11 @@ optional = true
version = "0.0.17"
git = "https://github.com/stellar/rs-soroban-env"
package = "soroban-env-host"
rev = "2069b9c10541126f7812c6f807006dfc8470ab3f"
rev = "c2e1c21cf8d44db23a159090e3cbaab741860295"

[dependencies.soroban-test-wasms]
git = "https://github.com/stellar/rs-soroban-env"
rev = "048be90e10dfda6486141f96ea86e32fb91681f4"
rev = "dd3d39571e9fdae9af34f7f46243d0794169b411"

[dependencies.cargo-lock]
git = "https://github.com/rustsec/rustsec"
Expand Down
Loading