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

Expiration Entry rework #137

Merged
merged 1 commit into from
Aug 23, 2023
Merged

Expiration Entry rework #137

merged 1 commit into from
Aug 23, 2023

Conversation

SirTyson
Copy link
Contributor

WIP Expiration Entry rework.

@2opremio
Copy link

2opremio commented Aug 17, 2023 •
edited
Loading

Splitting the data and expiration in separate ledger entries probably calls for reconsidering the XDR of the bump/restore footprint operations.

The ledger entries to bump/restore were implicitly supplied through keys in the footprint from the soroban transaction data. But now I guess that the final (i.e. preflighted) footprint will contain two entry keys (one for the data and one for the expiration), making it (even more) odd to use it as an implicit parameter.

I would suggest to add the ContractData/ContractCode keys explicitly to the operation bodies. As a side effect this will allow us to restrict the keys passed to ContractData/ContractCode through typing (e.g. defining a new union, ExpirableLedgerEntryKey, which only allows passing those keys).

@2opremio
Copy link

2opremio commented Aug 17, 2023 •
edited
Loading

In fact, we may even want to reconsider the bump/restore operation names. Maybe BumpContractLedgerEntries and RestoreContractLedgerEntries would make more sense if we decided not to use the footprint as input anymore.

@dmkozh
Copy link
Contributor

dmkozh commented Aug 17, 2023

I would suggest to add the ContractData/ContractCode keys explicitly to the operation bodies.

This PR in no way changes the reason for using the footprint for these ops, namely the need for consistency between how the footprints are treated for different operations. But whether expiration entries are present in the footprint is still up to debate.

@SirTyson
Copy link
Contributor Author

I would suggest to add the ContractData/ContractCode keys explicitly to the operation bodies.

This PR in no way changes the reason for using the footprint for these ops, namely the need for consistency between how the footprints are treated for different operations. But whether expiration entries are present in the footprint is still up to debate.

Now that we've hashed out a concurrency model, ContractData/ContractCode keys need to be in the RO set for bumps and restores, the associated ExpirationEntry key should not be present in the footprint.

@dmkozh
Copy link
Contributor

dmkozh commented Aug 18, 2023

Now that we've hashed out a concurrency model, ContractData/ContractCode keys need to be in the RO set for bumps

Just to add a bit more on that, they have to be in the footprint in order to guarantee the correct data dependency semantics (I don't think we have the time/need to move bump ops to a separate stage).

@SirTyson SirTyson marked this pull request as ready for review August 22, 2023 23:23
@Shaptic Shaptic mentioned this pull request Aug 23, 2023
Merged
@sreuland sreuland mentioned this pull request Aug 23, 2023
Closed
@SirTyson SirTyson force-pushed the expiration-entry branch 2 times, most recently from ad10fb2 to f521e32 Compare August 23, 2023 18:34
@dmkozh dmkozh merged commit 8a65f61 into stellar:next Aug 23, 2023
1 check passed
sisuresh added a commit that referenced this pull request Sep 8, 2023
@sisuresh @stellar-terraform
@graydon @leighmcculloch @jonjove @marta-lokhova @jayz22 @dmkozh @paulbellamy @SirTyson @brson
Next into curr (#147)
9ac0264 
* Initial commit

* Create check.yml (#2)

Defines the 'complete' status check.

* populate (#1)

* Incorporate CAP-42 changes (#3)

* Pick up Stellar-internal.x (#4)

* add .gitignore (#5)

* Absorb changes from rs-stellar-xdr repo (#7)

* Flatten contract spec discriminants (#8)

* Namespace the contract spec types (#9)

* Delete Stellar-contract-meta.x (#12)

* CAP-0047 - update names (#6)

* Fix contract spec definitions after rename (#13)

* Add contract meta types (#10)

* Rename BINARY to BYTES (#16)

* Add SCContractCode and update related types (#17)

* Add names to function inputs in contract spec (#18)

* Pick up persistent state changes (#19)

* Fix repeated errorCode in SCStatus (#20)

* Pick up changes incorrectly committed to rs-stellar-xdr (#21)

* Add rest of CAP-0056 (#22)

* Add BytesN as type (#23)

* Add Val as a spec type (#25)

* Add name of lib to struct and union spec entries (#26)

* Add adverts & demands to StellarMessage (#24)

* Add SST_CONTRACT_ERROR to SCStatusType (#29)

* Remove `ScHash` and `PublicKey` from object type. (#31)

* Add enum as a contract spec type (#34)

* Add error enum as a distinct UDT type from enum (#35)

* Add SCVal success to INVOKE_HOST_FUNCTION_SUCCESS, fix stellar/stellar-protocol#1307 (#36)

* Updated xdr for token wrapper and source auth (#37)

* add ENVELOPE_TYPE_CONTRACT_ID_FROM_ASSET

* add new HostFunction type and HashIDPreimage for using the source account in the host

* add missing enum value (#38)

* rename HOST_FN_CREATE_CONTRACT (#39)

* Add invoker to contract spec (#40)

* Add account id as object type (#41)

* Rename HOST_FN_CREATE_CONTRACT_WITH_SOURCE to HOST_FN_CREATE_CONTRACT_WITH_SOURCE_ACCOUNT (#42)

* Rename ENVELOPE_TYPE_CONTRACT_ID_FROM_SOURCE to ENVELOPE_TYPE_CONTRACT_ID_FROM_SOURCE_ACCOUNT (#43)

* Rename sourceContractID to sourceAccountContractID (#44)

* Add AccountID as type to contract spec (#45)

* move AccountID to avoid circular reference (#46)

* Add new host functions to xdr (#47)

* Add new host functions to xdr

* rename

* Trivial whitespace changes to Stellar-overlay.x to sync with curr (#50)

* XDR side of the contract deployment changes specified in CAP-46-02. (#51)

* XDR side of the contract deployment changes specified in CAP-46-02.

This introduces the ledger entries to store the wasm code and also changes the host functions to support creating/installing the contracts.

* Remove option to create and install contract in the same operation.

This relies on the ability to include multiple Soroban operations in the transaction. We can bring this back if we decide against that.

* Replace BigInt object with {iu}128 (#57)

* Bigint to int128 cont'd -- spec changes (#58)

* Migrate SC_SPEC from BIG_INT to {IU}128

* Make events a list of lists so we can associate events with their operation (#59)

* Make events a list of lists so we can associate events with their operation

* Add a OperationEvents struct

* Update overlay XDR (#62)

* XDR changes for Auth Next. (#65)

- Introduce the `ScAddress` type to generically represent the address in contracts
- Introduce structured authorization data and structured signature payload to use for auth in contracts

* Add fields for docs to the contract spec (#66)

* Make unit/void case explicit and support more tuples in union spec (#67)

* Make unit/void case explicit and support more tuples in union spec

* avoid void

* Fix syntax (#68)

* Add auth errors (#69)

* Add auth errors

* fixup! Add auth errors

* Add nonce to the signature payload (#71)

* Preliminary "remove objects from XDR" change (#70)

* Add diagnosticEvents (#74)

* XDR for upgradeable config entries (#75)

* Generalize configuration-related XDR.

The changes here are summarized in proposed updated to CAP-47 (stellar/stellar-protocol#1291).

- Use unique keys to identify settings
- Only distribute a hash for config upgrades in `StellarValue`
- Add SCP messages for exchanging config upgrade sets

* Remove unused ConfigSetting

---------

Co-authored-by: Dmytro Kozhevin <[email protected]>

* Fix U/I128 and U/I256 number representation for consistent sorting (#78)

* Fix 128 and 256 bits number representation

* fixup! Fix 128 and 256 bits number representation

* Add fee and limit-related configuration XDR based on CAP-0046-07. (#79)

Co-authored-by: Graydon Hoare <[email protected]>

* add config setting for host logic version (#80)

* Meta xdr (#82)

* Add XDR types for contract meta

* generic

* Update Stellar-contract-meta.x

Co-authored-by: Leigh McCulloch <[email protected]>

* Update Stellar-contract-meta.x

* Update Stellar-contract-meta.x

---------

Co-authored-by: Leigh McCulloch <[email protected]>

* Define config settings for contract costs (#81)

* Define cost parameters

* fixup! Define cost parameters

* Move config settings to separate file

* Remove spaces

* Add stellar namespace

* Transaction changes to support Soroban fee model. (#84)

* Transaction changes to support Soroban fee model.

- Moved all the resources (including the footprint) to the transaction level.
- Added host fn batching to compensate for removal of multi-operation tx support
- Did some passing-by naming cleanup (as we never really have time for that)

* Back out the host logic version config setting, will not use it (#85)

* Change cost param type to int64 (#87)

* Add config settings for contract data limits (#90)

* Add `txSOROBAN_RESOURCE_LIMIT_EXCEEDED` to the respective structs. (#91)

* Remove all TransactionResultSetV2 related changes and puts the hash of the events and return values in InvokeHostFunctionResult (#83)

* Remove all TransactionResultSetV2 related changes and puts the hash of TxHashOfMetaHashesSet in the new LedgerHeaderExtensionV2

* Remove hashing of tx meta components

* Remove nested events now that we only have one op per tx

* Return hash of events and return values in InvokeHostFunctionResult

* Err reform (#92)

* Remove comment with improbable suggestion

* Simplify error codes

* update overlay xdr (#94)

* update overlay xdr

* Add comment

* Replace ContractAuth addressWithNonce option with explicit union (#88)

* Replace ContractAuth addressWithNonce option with explicit union (#88)

* Move ContractAuth signatureArgs into Authorization (#89)

* Remove trailing , (#97)

* Changes to auth-related XDR and a bit of cleanup. (#95)

* Changes to auth-related XDR and a bit of cleanup.

- Prepare create contract host fn for using auth next
- Get rid of unnecessary envelopes
- Passing-by cleanup: get rid of `SCVAL_LIMIT`.

* Rename `HostFunctionArgs` back to just `HostFunction`.

* Use `ScAddress` to identify contracts in auth payload.

* Revert invoke host fn return value to be a single value. (#98)

* One more update to return value. (#99)

* State expiration (#93)

* Adds support for state expiration

* Cleanup and refactoring

* Added rent fees and revert footprint changes

* Get rid of floating point math for rent fees

* Add changes for env

* Adapt names to recent consensus on state expiration terminology

* Add state expiration related entries to LedgerCloseMeta as v2

* Move extension point to front of structure.

---------

Co-authored-by: Graydon Hoare <[email protected]>
Co-authored-by: Siddharth Suresh <[email protected]>

* Make `returnValue` optional in meta. (#100)

* Make `returnValue` optional in meta.

This is meaningless for non-Soroban txs.

* Make whole soroban tx meta optional.

Also did some small extension point cleanup.

* Add execution lanes config (#101)

* Add tx limit

* Minor naming tweak

* Small XDR fix. (#102)

* Add costs for new cryptography host functions (#105)

* Switch Soroban nonces from autoincrement to random values with signature expiration. (#103)

* Expiration bump op (#106)

* bump op

* Update thresholds

* Remove mergeable (#108)

* back out ContractCostType::VerifyEcdsaSecp256k1Sig, keeping Recover (#109)

* Add cost types for int256 arithmetics (#110)

* Add footprint to bump expiration name (#111)

* Remove Op postfix for result code (#113)

* Add support for contract instance storage. (#115)

- Store contract instance that includes executable and storage map under a special key
- Make contract executable separate from ScVal

* Add `VmCachedInstantiation` for future use (#116)

* Update XDR comments (#117)

* Remove ContractDataType from ScVal and rename (#118)

* Adds expiration iterator (#104)

* Adds expiration iterator

* Add BucketList size window and remove eviction iterator

* XDR for RestoreFootprintOp (#120)

* XDR for RestoreFootprintOp

* Use extension point instead

* Replace 'restorable' -> 'persistent' for state expiration config to match new terminology (#114)

* Fix comment (#121)

* Add error when accessing expired entry (#122)

* Refactor config XDR to account for dynamic write fees. (#123)

- Remove the flat fee
- Rename the fields that contribute to the write fee for clarity

* Soroban tx set size upgrade support and XDR cleanup. (#124)

- Use dedicated struct for `InvokeContract` host fn
- Replace `ScVec` with `SCVal` or `SCVal<>` depending on the context

* Make SCError into a union to allow user errors to be u32 (#125)

* Remove incorrect comment about Soroban u256 representation (#126)

* Change metadata fee config to only account for contract events. (#127)

Also renamed a couple tx-size related fields for clarity.

* Update `SorobanResources` for consistency with config update. (#128)

* One more metadata->events rename. (#129)

* Adds eviction iterator (#130)

* Remove `ContractCostType::GuardFrame` (#131)

* Clean up and clearify some `ContractCostType`s (#133)

* Add an error for exceeding refundable fee. (#134)

* Add errors for exceeding the refundable fee.

These have to be operation errors, as core is hard-wired to only return `txFAILED` on transaction failure and modifying this behavior is risky/slow.

* Remove `contractEventsSizeBytes` from `SorobanResources`.

This field is almost redundant and only helps to avoid apply-time error in the rare cases when the user sets the events resource, but forgets to set high enough refundable fee. It does, on the other hand, introduce another apply-time failure condition if the contracts emits a bit more events than expected (even if the refundable fee is high enough to cover that).

The total size of the events emitted is still governed by the network setting.

* Remove ScSpecTypeSet and SC_SPEC_TYPE_SET (#136)

* Expiration Entry rework (#137)

* typo (#139)

* expand SCError comments, no functional change (#140)

* expand SCError comments, no functional change

* Update Stellar-contract.x

Co-authored-by: Siddharth Suresh <[email protected]>

---------

Co-authored-by: Siddharth Suresh <[email protected]>

* rename txSOROBAN_RESOURCE_LIMIT_EXCEEDED to more general txSOROBAN_INVALID (#143)

* Update .github/workflows/check.yml

Co-authored-by: Graydon Hoare <[email protected]>

---------

Co-authored-by: stellar-terraform <[email protected]>
Co-authored-by: Graydon Hoare <[email protected]>
Co-authored-by: Leigh McCulloch <[email protected]>
Co-authored-by: jonjove <[email protected]>
Co-authored-by: mlo <[email protected]>
Co-authored-by: Hidenori <[email protected]>
Co-authored-by: Jay Geng <[email protected]>
Co-authored-by: Dmytro Kozhevin <[email protected]>
Co-authored-by: Paul Bellamy <[email protected]>
Co-authored-by: mlo <[email protected]>
Co-authored-by: Garand Tyson <[email protected]>
Co-authored-by: Jay Geng <[email protected]>
Co-authored-by: Brian Anderson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsachiherman tsachiherman tsachiherman left review comments

@2opremio 2opremio 2opremio left review comments

@Shaptic Shaptic Shaptic left review comments

@dmkozh dmkozh dmkozh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SirTyson @2opremio @dmkozh @Shaptic @tsachiherman