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

Add fee and limit-related configuration XDR #79

Merged
merged 2 commits into from
Apr 11, 2023
Merged

Add fee and limit-related configuration XDR #79

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca0c1cb
Add fee and limit-related configuration XDR based on CAP-0046-07.
dmkozh Apr 10, 2023
b06febd
Merge branch 'next' into fees_and_limits
graydon Apr 11, 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
99 changes: 98 additions & 1 deletion Stellar-ledger-entries.x
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,15 +505,112 @@ struct ContractCodeEntry {
opaque code<SCVAL_LIMIT>;
};

// Identifiers of all the network settings.
enum ConfigSettingID
{
CONFIG_SETTING_CONTRACT_MAX_SIZE_BYTES = 0
CONFIG_SETTING_CONTRACT_MAX_SIZE_BYTES = 0,
Copy link

Choose a reason for hiding this comment

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

I wonder if we should drop the prefix CONFIG_SETTING_ in the enum entries for better readability. These entries are within the enum ConfigSettingID so prefix feels redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently XDR gets compiled into an enum (i.e. not into enum class) hence we need to 'namespace' these in such fashion.

CONFIG_SETTING_CONTRACT_COMPUTE_V0 = 1,
CONFIG_SETTING_CONTRACT_LEDGER_COST_V0 = 2,
CONFIG_SETTING_CONTRACT_HISTORICAL_DATA_V0 = 3,
CONFIG_SETTING_CONTRACT_META_DATA_V0 = 4,
CONFIG_SETTING_CONTRACT_BANDWIDTH_V0 = 5
};

// "Compute" settings for contracts (instructions and memory).
struct ConfigSettingContractComputeV0
{
// Maximum instructions per ledger
int64 ledgerMaxInstructions;
Copy link

Choose a reason for hiding this comment

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

Should we use uint64 for this and the other two below since these will always be non-negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't worry much about representing non-negative values via unsigned values (especially in C++). But here the main thing is that the transaction XDR typically doesn't expose uint64 parameters (IIUC for better compatibility with clients such as javascript) and these values will be compared to the transaction values. I don't feel super strongly about this, but it at least seems a bit more consistent with the existing XDR definitions.

Copy link
Member

Choose a reason for hiding this comment

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

transaction XDR typically doesn't expose uint64 parameters (IIUC for better compatibility with clients such as javascript)

Fwiw int64 also exceeds the limits of JavaScript runtimes for what can be stored in a Number since a Number can only store lossless a 53-bit number and so avoiding uint64 in favor of int64 doesn't address the JavaScript issue. JavaScript also has BigInt now which alleviates the concern.

However, I've always appreciated the use of signed integers for any length or size values because it avoids underflow bugs.

// Maximum instructions per transaction
int64 txMaxInstructions;
// Cost of 10000 instructions
int64 feeRatePerInstructionsIncrement;

// Memory limit per contract/host function invocation. Unlike
// instructions, there is no fee for memory and it's not
// accumulated between operations - the same limit is applied
// to every operation.
uint32 memoryLimit;
};

// Ledger access settings for contracts.
struct ConfigSettingContractLedgerCostV0
{
// Maximum number of ledger entry read operations per ledger
uint32 ledgerMaxReadLedgerEntries;
// Maximum number of bytes that can be read per ledger
uint32 ledgerMaxReadBytes;
// Maximum number of ledger entry write operations per ledger
uint32 ledgerMaxWriteLedgerEntries;
// Maximum number of bytes that can be written per ledger
uint32 ledgerMaxWriteBytes;

// Maximum number of ledger entry read operations per transaction
uint32 txMaxReadLedgerEntries;
// Maximum number of bytes that can be read per transaction
uint32 txMaxReadBytes;
// Maximum number of ledger entry write operations per transaction
uint32 txMaxWriteLedgerEntries;
// Maximum number of bytes that can be written per transaction
uint32 txMaxWriteBytes;

int64 feeReadLedgerEntry; // Fee per ledger entry read
int64 feeWriteLedgerEntry; // Fee per ledger entry write

int64 feeRead1KB; // Fee for reading 1KB
int64 feeWrite1KB; // Fee for writing 1KB

// Bucket list fees grow slowly up to that size
int64 bucketListSizeBytes;
// Fee rate in stroops when the bucket list is empty
int64 bucketListFeeRateLow;
// Fee rate in stroops when the bucket list reached bucketListSizeBytes
int64 bucketListFeeRateHigh;
// Rate multiplier for any additional data past the first bucketListSizeBytes
uint32 bucketListGrowthFactor;
};

// Historical data (pushed to core archives) settings for contracts.
struct ConfigSettingContractHistoricalDataV0
{
int64 feeHistorical1KB; // Fee for storing 1KB in archives
};

// Meta data (pushed to downstream systems) settings for contracts.
struct ConfigSettingContractMetaDataV0
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for TransactionMeta? I don't think we refer to "meta" as "metadata" anywhere, so this might be confusing. I would just remove the "data" portion of the name.

{
// Maximum size of extended meta data produced by a transaction
uint32 txMaxExtendedMetaDataSizeBytes;
// Fee for generating 1KB of extended meta data
int64 feeExtendedMetaData1KB;
};

// Bandwidth related data settings for contracts
struct ConfigSettingContractBandwidthV0
{
// Maximum size in bytes to propagate per ledger
uint32 ledgerMaxPropagateSizeBytes;
// Maximum size in bytes for a transaction
uint32 txMaxSizeBytes;

// Fee for propagating 1KB of data
int64 feePropagateData1KB;
};

union ConfigSettingEntry switch (ConfigSettingID configSettingID)
{
case CONFIG_SETTING_CONTRACT_MAX_SIZE_BYTES:
uint32 contractMaxSizeBytes;
case CONFIG_SETTING_CONTRACT_COMPUTE_V0:
ConfigSettingContractComputeV0 contractCompute;
case CONFIG_SETTING_CONTRACT_LEDGER_COST_V0:
ConfigSettingContractLedgerCostV0 contractLedgerCost;
case CONFIG_SETTING_CONTRACT_HISTORICAL_DATA_V0:
ConfigSettingContractHistoricalDataV0 contractHistoricalData;
case CONFIG_SETTING_CONTRACT_META_DATA_V0:
ConfigSettingContractMetaDataV0 contractMetaData;
case CONFIG_SETTING_CONTRACT_BANDWIDTH_V0:
ConfigSettingContractBandwidthV0 contractBandwidth;
};

struct LedgerEntryExtensionV1
Expand Down