State expiration

[SirTyson]

Description

Resolves #3697.

This provides initial support for state expiration. With this update, SQL and BucketListDB track entry entry lifetimes. Additionally, accessed entry lifetimes are automatically bumped, with an optional additional bump that can be defined in the footprint. While the BucketListDB behavior has been tested, I cannot run lifetime extension unit tests until this XDR change is merged and the state expiration interface updates have been merged on the Rust side. Until these changes and the unit tests pass, I'm leaving this as a draft, but there should be plenty to review in the mean time.

This PR does not enforce expiration, which means entries are accessible after they have expired and are not deleted. Additionally, while lifetime extensions are implemented, we do not yet charge any rent related fees.

Checklist

• Reviewed the contributing document
• Rebased on top of master (no merge commits)
• Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
• Compiles
• Ran all tests
• If change impacts performance, include supporting evidence per the performance document

[dmkozh]

This is the first pass of review for stuff outside of bucketlist changes. There is nothing too problematic, but we do need to add fee-related and validation-related stuff (might actually be ok to do that in a separate PR - this really depends on the amount of changes).

[dmkozh]

nit: that too short and not really readable, please use a more descriptive name, like footprintEntry. Also maybe a better interface would be a vec-to-vec converter (as you always deal with vecs for footprints).

[dmkozh]

This is a bit hard to follow, would it be possible to break this down a bit (e.g. move lambdas out into functions and factor out some stuff in for (auto const& fe : resources.footprint.readOnly) loop).

[dmkozh]

I suppose this works fine because you call this only on bumpable entries, but a safer check would be data.type() == CONTRACT_CODE || (type == CONTRACT_DATA && <your check>)

[dmkozh]

This part is actually kind of tricky. I think the right thing to do here is to charge the whole refundableFee as a part of resource fee and then do the refund for the unspent part. You'll also need to fail if you run out of fee.

[dmkozh]

I'm not sure if this is correct - can't this cause exceeding the fee? Maybe it's safer to not do the bump at all?

[SirTyson]

We have to enforce the minimum expiration to prevent rent gaming since we can't actually start expiring UNIQUE and RESTORABLE entries until level 6 of the BucketList. This can cause exceeding the fee, but the thinking is that since rent payments are coming out of refundable fee, users should be able to be overly cautious and put in a little extra XLM for cases such as this. However we only need to enforce this on writes, not for the read-only entries. I think currently this applies the minimum to both read only and writes, so I'll change that, but keep the minimum as it is for modified entries.

[dmkozh]

nit: I'm not sure this is the right place for LedgerEntry utils - somewhere in ledger folder would be more appropriate.

[dmkozh]

Nit: you can move this into if (success) below.

[dmkozh]

Can we do any pre-apply validation, like verify if bumps can be covered by the fees (maybe roughly?)

[dmkozh]

Would it make sense to fail at validation step? It doesn't seem hard to avoid specifying too high expiration ledger.

[SirTyson]

We shouldn't do this because the maxLedgerLifetime is a network config parameter that can change. For example, suppose a contract correctly bumps all entries it touches to the maximum rent at the time of 1 year. After the contract is deployed, the network config changes and the maximum lifetime is no 6 months. This previously valid contract is now invalid on every contract call if we fail at the validation step if lifetime bumps are too high

[dmkozh]

This is the first pass of review for stuff outside of bucketlist changes. There is nothing too problematic, but we do need to add fee-related and validation-related stuff (might actually be ok to do that in a separate PR - this really depends on the amount of changes).