diff --git a/libraries/sysiolib/contracts/sysio/kv_cached.hpp b/libraries/sysiolib/contracts/sysio/kv_cached.hpp new file mode 100644 index 000000000..e5884ec68 --- /dev/null +++ b/libraries/sysiolib/contracts/sysio/kv_cached.hpp @@ -0,0 +1,320 @@ +#pragma once +/** + * sysio::kv::cached_value -- write-deferring cache over a KV singleton store. + * + * Loads the stored value at most once and serves every read from that cache. Each pending change + * is written back exactly once, by flush() or by destruction, and only when a mutating call + * actually ran -- so an action that only reads never writes. Note that flush() RE-ARMS the handle: + * it commits the pending change and clears it, so a later mutation owes a second write. N explicit + * flush/mutate cycles therefore produce N writes; leaving the commit to the destructor produces + * exactly one per action. + * + * WHY THIS EXISTS + * + * The classic singleton idiom caches state in a contract member and persists it from the + * contract destructor: + * + * struct my_contract : sysio::contract { + * my_contract(...) { _gstate = _global.get(); } + * ~my_contract() { _global.set(_gstate, get_self()); } // unconditional write + * }; + * + * That destructor runs after EVERY action, because the generated dispatcher instantiates + * the contract as a temporary and destroys it as soon as the action returns. So every + * action -- including one that only reads -- issues a kv_set, and the chain rejects any + * write inside a read-only transaction: + * + * cannot store a KV record when executing a readonly transaction + * + * The failure is confusing because it surfaces only on the SUCCESS path: an action that + * aborts early via sysio::check traps before the destructor runs and reports its own + * error, while an action that returns normally dies in the destructor, making it look + * like the return path is at fault. + * + * cached_value removes the hazard by construction rather than by remembering to guard: an + * action that only reads never dirties the cache, so it never issues a kv_set, so it is + * legal inside a read-only transaction. + * + * STORE REQUIREMENTS + * + * Any singleton-shaped store works. It must expose: + * + * using value_type = T; + * bool try_get(value_type& out) const; // false when absent + * void set(const value_type& val, name payer); + * void remove(); + * + * Both sysio::kv::global (unscoped) and sysio::kv_singleton (scoped) satisfy this; see + * kv::cached_global and sysio::cached_kv_singleton for the ready-made aliases. + * + * VISIBILITY + * + * Nothing reaches the store before flush(). A second handle opened on the same table + * during the same action therefore does NOT observe uncommitted changes -- exactly as a + * contract-member cache behaves today. Keep one cached handle per table per action, and + * do not mix it with direct writes to the same table. Inline actions are unaffected: they + * are queued during apply() and executed after it returns, so the flush always lands + * first. + * + * Usage: + * kv::cached_global<"global"_n, my_state> _global{get_self()}; + * + * const auto& s = _global.get(); // read, no write + * _global.modify(get_self(), [](auto& s) { // deferred + * s.counter++; + * }); + * // written back exactly once, at destruction + */ + +#include +#include + +#include +#include +#include +#include + +namespace sysio { namespace kv { + +/** + * Write-deferring cache over a singleton-shaped KV store. + * + * @tparam Store backing store satisfying the requirements documented above. + */ +template +class cached_value { +public: + /// Payload type, taken from the backing store so it never has to be repeated. + using value_type = typename Store::value_type; + +private: + using T = value_type; + + /// The single change owed to the store, applied by flush(). + enum class pending_op : uint8_t { + none, ///< nothing to write back + write, ///< _cache must be stored + erase ///< the row must be erased + }; + +public: + /** + * Construct a handle. All arguments are forwarded to the backing store, so this works + * over both the unscoped store (code) and the scoped one (code, scope). + */ + /// The enable_if keeps this from outcompeting the deleted copy constructor when the + /// argument is a non-const cached_value lvalue, so copying reports "deleted" rather than + /// a confusing failure inside the store's constructor. + template, cached_value>...>>> + explicit cached_value(Args&&... args) : _store(std::forward(args)...) {} + + /// Non-copyable: two handles on one table would each own a conflicting pending write. + cached_value(const cached_value&) = delete; + cached_value& operator=(const cached_value&) = delete; + + /// Applies the pending change, if any. + ~cached_value() { flush(); } + + /// True if the row exists, accounting for a pending set()/remove(). + bool exists() const { + load(); + return _present; + } + + /** + * Cached value. Asserts if the row is absent. + * + * Returns a REFERENCE into the cache, where kv::global::get() and kv_singleton::get() both + * return by value. Binding it (`const auto& s = h.get();`) therefore aliases the live cache + * rather than taking a snapshot: a later modify()/set()/modify_or_create() through this handle + * is visible through \p s. That is intentional -- copying a singleton payload on every read is + * what this class exists to avoid -- but it means a reference held across a mutation reports + * the new value, not the value that was read. Copy it if you need a snapshot. remove() keeps + * the cached object alive precisely so an outstanding reference never dangles. + * + * @param msg assert message used when absent. + */ + const T& get(const char* msg = "singleton does not exist") const { + load(); + sysio::check(_present, msg); + return *_cache; + } + + /// Cached value, or \p def when absent. Never creates the row and never dirties. + T get_or_default(const T& def) const { + load(); + return _present ? *_cache : def; + } + + /** + * Mutate the value in place. Asserts if the row is absent -- use modify_or_create() to + * create. The write is deferred to flush()/destruction. + * + * @param payer account billed for the row when the change is written back. + * @param f callable receiving a mutable reference to the cached value. + */ + template + void modify(sysio::name payer, Lambda&& f) { + load(); + // Distinguish "never existed" from "you erased it earlier in this action". Without this the + // second case reports the first case's message, which sends the reader hunting for a missing + // row that their own remove() retired. + sysio::check(_pending != pending_op::erase, "singleton mutated after remove()"); + sysio::check(_present, "singleton does not exist"); + mutate(payer, std::forward(f)); + } + + /** + * Mutate the value in place, seeding the cache from \p def first when the row is absent. + * The write is deferred to flush()/destruction. + * + * NOT named upsert, deliberately. kv::table::upsert(payer, key, default_value, updater) + * stores default_value VERBATIM on the insert path and never invokes the updater there -- + * callers pass a fully-populated default and treat the lambda as update-only. This does the + * opposite: \p def only seeds the cache and \p f runs in every case, so a blank default plus + * a lambda that fills it in is the idiomatic call. Two functions in sysio::kv with one name + * and opposite insert semantics would be applied interchangeably by mistake, and the + * mistake is silent -- the row is written either way, just with different contents. + */ + template + void modify_or_create(sysio::name payer, const T& def, Lambda&& f) { + load(); + // Checked BEFORE seeding: on the erase path this call is rejected, and it must not leave the + // handle holding a seeded value it never gets to write. + sysio::check(_pending != pending_op::erase, "singleton mutated after remove()"); + if (!_present) { + _cache = def; + _present = true; + } + mutate(payer, std::forward(f)); + } + + /** + * Materialize \p def into the cache when the row is absent, WITHOUT owing a write. + * + * This is the safe form of "give me defaults on a chain where nobody has written the row yet". + * Seeding through set()/modify_or_create() would mark the handle dirty, so every action -- + * including a pure query -- would flush a kv_set and be refused inside a read-only transaction, + * which is the exact failure this class exists to prevent. After seeding, reads see \p def and + * the defaults reach storage only when an action genuinely mutates something. + */ + void seed_if_absent(const T& def) { + load(); + if (!_present) { + _cache = def; + _present = true; + } + } + + /** + * Replace the value outright. Creates the row if absent. Deferred. + * + * Passing kv::same_payer keeps whatever payer this handle already recorded, so set(v, payer) + * followed by set(w, same_payer) still bills the one coalesced write to payer -- see + * record_payer(). + */ + void set(const T& val, sysio::name payer) { + _loaded = true; + _present = true; + _cache = val; + record_payer(payer); + _pending = pending_op::write; + } + + /** + * Erase the row, discarding any pending write. Deferred. + * + * Probes the store first so that erasing a row that is not there costs nothing and stays legal + * inside a read-only transaction. Without the probe, both backing stores short-circuit an erase + * of an absent row at flush time, so whether this action issued a write -- and therefore whether + * it was legal read-only -- would depend on chain data rather than on the code. + * + * The cached object is deliberately NOT destroyed: _present already records the row as gone, and + * keeping it alive means a reference handed out by an earlier get() never dangles. + * + * Idempotent: calling this twice owes the same single erase as calling it once. + */ + void remove() { + load(); + if (!_present) { + // Absent per the cache -- but WHY it is absent decides what is owed. An erase this handle + // already recorded is the reason _present is false, and cancelling it here would leave the + // stored row in place while the handle went on reporting it gone. Only a row that was + // never there discards the debt, and there the debt can only be a pending write. + if (_pending != pending_op::erase) _pending = pending_op::none; + return; + } + _present = false; + _pending = pending_op::erase; + } + + /// True when a change is owed to the store. + bool dirty() const { return _pending != pending_op::none; } + + /// Apply the pending change, if any. Idempotent. + void flush() { + const auto op = _pending; + // Cleared before the store call: a rejected write aborts the transaction anyway, so + // there is no state to retry, and this keeps a manual flush() followed by the + // destructor from attempting the same write twice. + _pending = pending_op::none; + switch (op) { + case pending_op::write: _store.set(*_cache, _payer); break; + case pending_op::erase: _store.remove(); break; + case pending_op::none: break; + } + } + +private: + /// Shared tail of modify()/modify_or_create(): apply \p f and record the debt. + template + void mutate(sysio::name payer, Lambda&& f) { + sysio::check(_pending != pending_op::erase, "singleton mutated after remove()"); + f(*_cache); + // Re-checked AFTER f(): the callback holds a reference to this handle's cache and may call + // remove() through it. Recording a write here would convert that erase back into a store of + // the value the caller just retired, and the caller would get no diagnostic. + sysio::check(_pending != pending_op::erase, "singleton removed from inside a mutation callback"); + record_payer(payer); + _pending = pending_op::write; + } + + /** + * Record the account to bill for the pending write. + * + * A default-constructed name is kv::same_payer -- "bill whoever already owns the row" -- so it + * must never displace a real payer recorded earlier in this action. Every call that dirties the + * handle coalesces into a SINGLE kv_set, and the host rejects payer 0 when that kv_set creates + * the row; uncached, the create and the update were separate writes and only the update could + * legally carry same_payer. With no real payer recorded, 0 reaches the host untouched, which is + * what makes an update keep its existing payer and a create fail exactly as it does uncached. + * + * Every path that dirties the handle records its payer HERE rather than assigning _payer + * directly, so a new write path cannot reintroduce the divergence by forgetting the rule. + */ + void record_payer(sysio::name payer) { + if (payer.value != 0) _payer = payer; + } + + /// Populate the cache from the store. At most one store read per handle. + void load() const { + if (_loaded) return; + _loaded = true; + T val; + if (_store.try_get(val)) { + _cache = std::move(val); + _present = true; + } + } + + Store _store; + mutable std::optional _cache; + sysio::name _payer{}; + mutable bool _loaded = false; ///< has the store been consulted yet + mutable bool _present = false; ///< does the row exist, per cache + pending_op _pending = pending_op::none; +}; + +}} // namespace sysio::kv diff --git a/libraries/sysiolib/contracts/sysio/kv_global.hpp b/libraries/sysiolib/contracts/sysio/kv_global.hpp index c76a1290b..b6a0dee5e 100644 --- a/libraries/sysiolib/contracts/sysio/kv_global.hpp +++ b/libraries/sysiolib/contracts/sysio/kv_global.hpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -33,6 +34,11 @@ template class global { static_assert(std::is_default_constructible_v, "global value type must be default constructible"); +public: + /// Payload type. Lets generic wrappers (kv::cached_value) deduce it without repetition. + using value_type = T; + +private: static constexpr uint32_t _table_id = sysio::kv::compute_table_id(static_cast(Name)); uint64_t _code = 0; @@ -84,14 +90,26 @@ class global { return def; } -private: /// Single kv_get call — returns true if found, populates \p out. + /// Public so generic wrappers (kv::cached_value) load through this single-kv_get path + /// instead of paying kv_contains followed by kv_get. bool try_get(T& out) const { auto k = make_key(); if constexpr (is_fixed_serializable_v) { char vbuf[sizeof(T)]; int32_t sz = ::kv_get(_table_id, code(), k.data, key_size, vbuf, sizeof(T)); if (sz < 0) return false; + // kv_get fills min(buffer, stored) bytes but returns the FULL stored size, so a row that is + // not exactly sizeof(T) must be rejected rather than memcpy'd. Copying sizeof(T) out of a + // partially filled buffer splices whatever this contract's linear memory happened to hold + // into the payload, so the caller silently gets stale bytes where it expects stored data. + // This is not a divergence risk -- linear memory is deterministic, so every node builds the + // same wrong value -- but a wrong value that reads as authoritative is worse than a trap. A + // size mismatch means the row was written by an incompatible version of this type, so trap + // rather than report "absent": silently treating a real row as missing invites the caller + // to overwrite it. + sysio::check(sz == static_cast(sizeof(T)), + "kv::global: stored value size does not match the fixed-serializable payload"); std::memcpy(&out, vbuf, sizeof(T)); } else { char stack[kv_value_stack_size]; @@ -111,9 +129,16 @@ class global { return true; } -public: - - /// Stores or overwrites the value. + /** + * Stores or overwrites the value. + * + * WRITES IGNORE code(). kv_get and kv_contains take a code parameter, so reads honour whatever + * account this handle was constructed with; kv_set and kv_erase have no such parameter and + * always land on the current receiver. A handle opened on a FOREIGN account is therefore + * read-only in practice -- calling this on one reads their row and writes your own. Nothing + * detects that at compile time, and a runtime guard would cost a current_receiver() host call on + * every write, so it is left to the caller: construct foreign-code handles for reading only. + */ void set(const T& val, sysio::name payer) { auto k = make_key(); if constexpr (is_fixed_serializable_v) { @@ -138,4 +163,15 @@ class global { } }; +/** + * Write-deferring kv::global. + * + * Reads never issue a kv_set, so an action that only reads the singleton stays legal inside + * a read-only transaction. Use this in place of caching the value in a contract member and + * writing it back from the contract destructor. See kv_cached.hpp for the rationale and for + * the visibility rules that come with deferred writes. + */ +template +using cached_global = cached_value>; + }} // namespace sysio::kv diff --git a/libraries/sysiolib/contracts/sysio/kv_singleton.hpp b/libraries/sysiolib/contracts/sysio/kv_singleton.hpp index 37db7c15e..f3562229f 100644 --- a/libraries/sysiolib/contracts/sysio/kv_singleton.hpp +++ b/libraries/sysiolib/contracts/sysio/kv_singleton.hpp @@ -1,6 +1,10 @@ #pragma once #include "kv_multi_index.hpp" -#include "system.hpp" +#include "kv_cached.hpp" +// Deliberately does NOT include system.hpp: nothing here (nor in kv_multi_index.hpp or +// kv_table.hpp) uses any symbol it declares, and its is_feature_activated declaration collides +// with the C-API one, which made this header unusable from a native unit test that also includes +// the C API -- so the scoped alias below could not be covered by the natively-run suite. namespace sysio { @@ -23,6 +27,9 @@ namespace sysio { public: + /// Payload type. Lets generic wrappers (kv::cached_value) deduce it without repetition. + using value_type = T; + kv_singleton( name code, uint64_t scope ) : _t( code, scope ) {} bool exists() const { @@ -35,6 +42,19 @@ namespace sysio { return itr->value; } + /// Returns true and populates \p out when the row exists, in one call. + /// + /// One CALL, not one intrinsic: this goes through kv_multi_index::find, which costs + /// several billed host calls on the row-exists path. It exists so kv::cached_value has a + /// non-asserting load that does not pay exists()-then-get() on top of that, not because it + /// is cheap. (kv::global::try_get genuinely is a single kv_get.) + bool try_get( T& out ) const { + auto itr = _t.find( pk_value ); + if( itr == _t.end() ) return false; + out = itr->value; + return true; + } + T get_or_default( const T& def = T() ) const { auto itr = _t.find( pk_value ); return itr != _t.end() ? itr->value : def; @@ -67,4 +87,16 @@ namespace sysio { table _t; }; + /** + * Write-deferring kv_singleton. + * + * Reads never issue a kv_set, so an action that only reads the singleton stays legal + * inside a read-only transaction. This is the drop-in for a contract ported from the + * classic idiom of caching the value in a member and writing it back from the contract + * destructor -- that pattern makes every action, including pure queries, fail read-only + * execution. See kv_cached.hpp for the rationale and the deferred-write visibility rules. + */ + template + using cached_kv_singleton = kv::cached_value>; + } /// namespace sysio diff --git a/plugins/sysio/abigen.hpp b/plugins/sysio/abigen.hpp index ec09df5c1..5e7882a40 100644 --- a/plugins/sysio/abigen.hpp +++ b/plugins/sysio/abigen.hpp @@ -1228,6 +1228,21 @@ namespace sysio { namespace cdt { virtual bool VisitDecl(clang::Decl* decl) { if (const auto* d = dyn_cast(decl)) { + // kv::cached_value is a transparent wrapper: the table it stands for is the one + // its Store parameter describes. Unwrap to that Store, but keep the WRAPPER as the decl + // whose use marks the table as belonging to this contract -- the inner specialization is + // only ever named inside cached_value, so defined_in_contract() would reject it and a + // payload without [[sysio::table]] would silently lose its ABI table entry. Nothing + // fails at build time; it surfaces later as clio get table, SHiP, and the generated SDK + // types no longer seeing the table. + const clang::ClassTemplateSpecializationDecl* owner = d; + if (d->getName() == "cached_value" && d->getTemplateArgs().size() >= 1 && + d->getTemplateArgs()[0].getKind() == clang::TemplateArgument::Type) { + if (const auto* store = d->getTemplateArgs()[0].getAsType().getTypePtr()->getAsCXXRecordDecl()) { + if (const auto* store_spec = dyn_cast(store)) + d = store_spec; + } + } if (d->getName() == "multi_index" || d->getName() == "singleton" || d->getName() == "kv_multi_index" || d->getName() == "table" || d->getName() == "scoped_table" || d->getName() == "global") { @@ -1241,7 +1256,7 @@ namespace sysio { namespace cdt { const auto* key_type = d->getTemplateArgs()[1].getAsType().getTypePtr()->getAsCXXRecordDecl(); const auto* val_type = d->getTemplateArgs()[2].getAsType().getTypePtr()->getAsCXXRecordDecl(); auto val_decl = clang_wrapper::wrap_decl(val_type); - if ((val_decl.isSysioTable() && ag.is_sysio_contract(val_decl, ag.get_contract_name())) || defined_in_contract(d)) { + if ((val_decl.isSysioTable() && ag.is_sysio_contract(val_decl, ag.get_contract_name())) || defined_in_contract(owner)) { auto table_name_raw = d->getTemplateArgs()[0].getAsIntegral().getLimitedValue(); // Extract secondary index info from Indices... (args[3..]) @@ -1287,7 +1302,7 @@ namespace sysio { namespace cdt { // multi_index, singleton, kv_multi_index, global — arg[1] is value type const auto* table_type = d->getTemplateArgs()[1].getAsType().getTypePtr()->getAsCXXRecordDecl(); auto table_decl = clang_wrapper::wrap_decl(table_type); - if ((table_decl.isSysioTable() && ag.is_sysio_contract(table_decl, ag.get_contract_name())) || defined_in_contract(d)) { + if ((table_decl.isSysioTable() && ag.is_sysio_contract(table_decl, ag.get_contract_name())) || defined_in_contract(owner)) { const auto table_name_raw = d->getTemplateArgs()[0].getAsIntegral().getLimitedValue(); // Extract indexed_by<...> secondary indices for multi_index/kv_multi_index. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5b2308fdd..77dda74c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -22,6 +22,7 @@ add_unit_test( system_tests ) add_unit_test( time_tests ) add_unit_test( varint_tests ) add_unit_test( kv_table_tests ) +add_unit_test( kv_cached_tests ) add_test( NAME toolchain_tests COMMAND ${CMAKE_BINARY_DIR}/tools/toolchain-tester/toolchain-tester ${CMAKE_SOURCE_DIR}/tests/toolchain --cdt ${CMAKE_BINARY_DIR}/bin --verbose ) set_property(TEST toolchain_tests PROPERTY LABELS toolchain_tests) diff --git a/tests/integration/contracts.hpp.in b/tests/integration/contracts.hpp.in index 87d999dfc..d3e8daffb 100644 --- a/tests/integration/contracts.hpp.in +++ b/tests/integration/contracts.hpp.in @@ -64,6 +64,9 @@ namespace sysio::testing { static std::vector kv_global_tests_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/kv_global_tests.wasm"); } static std::vector kv_global_tests_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/kv_global_tests.abi"); } + static std::vector kv_cached_contract_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/kv_cached_contract.wasm"); } + static std::vector kv_cached_contract_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/kv_cached_contract.abi"); } + static std::vector hash_id_tests_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/hash_id_tests.wasm"); } static std::vector hash_id_tests_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/hash_id_tests.abi"); } }; diff --git a/tests/integration/kv_cached_tests.cpp b/tests/integration/kv_cached_tests.cpp new file mode 100644 index 000000000..851cd2b9d --- /dev/null +++ b/tests/integration/kv_cached_tests.cpp @@ -0,0 +1,139 @@ +/** + * @file + * @copyright defined in sysio.cdt/LICENSE.txt + * + * End-to-end coverage for kv::cached_global and sysio::cached_kv_singleton against a real + * chain, driving the kv_cached_tests contract. + * + * This is the test that pins the actual defect. A contract that caches singleton state in a + * member and writes it back unconditionally makes every action issue a kv_set, because the + * generated dispatcher destroys the contract instance as soon as the action returns. Inside + * a read-only transaction the chain rejects that write with + * + * cannot store a KV record when executing a readonly transaction (table_operation_not_permitted) + * + * and the failure only shows up on the SUCCESS path -- an action that aborts early via + * check() traps before the destructor runs and reports its own error instead. The cached + * types only write when an action actually mutated something, so a query action is legal + * read-only while a mutating one is still correctly refused. + */ + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wsign-compare" +#include +#pragma GCC diagnostic pop + +#include + +#include + +using namespace sysio; +using namespace sysio::chain; +using namespace sysio::testing; + +#ifdef NON_VALIDATING_TEST +#define TESTER tester +#else +#define TESTER validating_tester +#endif + +namespace { + +constexpr auto cached_acct = "kvcach"_n; + +/// Deploy the kv_cached_tests contract. +void deploy(TESTER& t) { + t.produce_blocks(1); + t.create_account(cached_acct); + t.produce_blocks(1); + t.set_code(cached_acct, contracts::kv_cached_contract_wasm()); + t.set_abi(cached_acct, contracts::kv_cached_contract_abi().data()); + t.produce_blocks(1); +} + +/// Push \p act as a read-only transaction. Read-only transactions carry no authorizations, +/// which is why the actions under test do not call require_auth. +transaction_trace_ptr push_readonly(TESTER& t, action_name act) { + signed_transaction trx; + trx.actions.push_back(t.get_action(cached_acct, act, {}, {})); + t.set_transaction_headers(trx); + return t.push_transaction(trx, fc::time_point::maximum(), TESTER::DEFAULT_BILLED_CPU_TIME_US, + false, transaction_metadata::trx_type::read_only); +} + +} // namespace + +BOOST_AUTO_TEST_SUITE(kv_cached_tests) + +/// A query action on a contract with member-cached singletons must succeed read-only. +BOOST_FIXTURE_TEST_CASE(kv_cached_readonly_query, TESTER) { try { + deploy(*this); + + // Reads that MISS are still reads: no row exists yet, and no write may be attempted. + push_readonly(*this, "roabsent"_n); + + push_action(cached_acct, "seed"_n, cached_acct, {}); + produce_blocks(1); + + // The regression itself: pure reads, contract instance destroyed on return, no kv_set. + push_readonly(*this, "roread"_n); + + // Reading read-only must not have mutated anything -- a normal push still sees the seed. + push_action(cached_acct, "roread"_n, cached_acct, {}); + + BOOST_REQUIRE_EQUAL(validate(), true); +} FC_LOG_AND_RETHROW() } + +/// A mutating action must still be refused inside a read-only transaction. Without this the +/// suite above could pass simply because writes stopped being enforced. +BOOST_FIXTURE_TEST_CASE(kv_cached_readonly_rejects_mutation, TESTER) { try { + deploy(*this); + push_action(cached_acct, "seed"_n, cached_acct, {}); + produce_blocks(1); + + BOOST_REQUIRE_THROW(push_readonly(*this, "bump"_n), table_operation_not_permitted); + + // The refused transaction must leave nothing behind. + push_action(cached_acct, "roread"_n, cached_acct, {}); + + BOOST_REQUIRE_EQUAL(validate(), true); +} FC_LOG_AND_RETHROW() } + +/// Deferred writes actually persist, and repeated mutations inside one action collapse into a +/// single stored result. +BOOST_FIXTURE_TEST_CASE(kv_cached_deferred_writes_persist, TESTER) { try { + deploy(*this); + + push_action(cached_acct, "seed"_n, cached_acct, {}); + produce_blocks(1); + + push_action(cached_acct, "bump"_n, cached_acct, {}); + produce_blocks(1); + push_action(cached_acct, "chkbump"_n, cached_acct, {}); + + // Five in-action mutations, one stored outcome: 43 + 5*10. + push_action(cached_acct, "multibump"_n, cached_acct, {}); + produce_blocks(1); + push_action(cached_acct, "chkmulti"_n, cached_acct, {}); + + push_action(cached_acct, "rmall"_n, cached_acct, {}); + produce_blocks(1); + + // Erased rows stay erased across transactions, and reading them remains read-only legal. + push_readonly(*this, "roabsent"_n); + + BOOST_REQUIRE_EQUAL(validate(), true); +} FC_LOG_AND_RETHROW() } + +/// modify_or_create() creates a row that was never seeded. +BOOST_FIXTURE_TEST_CASE(kv_cached_modify_or_create_creates, TESTER) { try { + deploy(*this); + + push_action(cached_acct, "createnew"_n, cached_acct, {}); + produce_blocks(1); + push_action(cached_acct, "chkcreate"_n, cached_acct, {}); + + BOOST_REQUIRE_EQUAL(validate(), true); +} FC_LOG_AND_RETHROW() } + +BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index db47ded54..d13093af7 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -34,6 +34,7 @@ add_cdt_unit_test(protobuf_wire_tests) add_cdt_unit_test(time_tests) add_cdt_unit_test(varint_tests) add_cdt_unit_test(kv_table_tests) +add_cdt_unit_test(kv_cached_tests) target_compile_options( rope_tests PUBLIC -g ) add_subdirectory(test_contracts) diff --git a/tests/unit/kv_cached_tests.cpp b/tests/unit/kv_cached_tests.cpp new file mode 100644 index 000000000..67082ff15 --- /dev/null +++ b/tests/unit/kv_cached_tests.cpp @@ -0,0 +1,880 @@ +/** + * @file + * @copyright defined in sysio.cdt/LICENSE.txt + * + * Coverage for sysio::kv::cached_value and its kv::cached_global alias (kv_cached.hpp). The + * scoped sysio::cached_kv_singleton alias is covered on chain by kv_cached_contract, driven from + * tests/integration/kv_cached_tests.cpp -- see the note above main(). + * + * The behaviour under test is WRITE SUPPRESSION. The classic singleton idiom caches state in + * a contract member and writes it back from the contract destructor, which the generated + * dispatcher runs after every action -- so even a pure query issues a kv_set, and the chain + * rejects that inside a read-only transaction with + * + * cannot store a KV record when executing a readonly transaction + * + * cached_value fixes this by only writing when a mutating call actually ran. Nearly every + * case below therefore asserts on an exact store-call COUNT, not just on the resulting value: + * a cache that happens to hold the right data but still writes on a read path would pass a + * value-only test and reintroduce the bug. + * + * Two backings are exercised: + * - counting_store, a minimal Store that records every call, for the generic semantics. + * - the real kv::global over mocked KV intrinsics, for integration (key encoding, both + * serialization paths, payer propagation). + */ + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace sysio; +using namespace sysio::native; + +namespace { + +// --------------------------------------------------------------------------- +// Payload types +// --------------------------------------------------------------------------- + +/// Fixed-serializable payload: no padding, so sizeof == pack_size and kv::global takes its +/// zero-copy memcpy path. +struct pod_state { + uint64_t counter = 0; + uint64_t flags = 0; + + SYSLIB_SERIALIZE(pod_state, (counter)(flags)) +}; +inline bool operator==(const pod_state& a, const pod_state& b) { + return a.counter == b.counter && a.flags == b.flags; +} +static_assert(sysio::kv::is_fixed_serializable_v, + "pod_state must exercise kv::global's fixed-serializable path"); + +/// Variable-length payload: forces kv::global's pack/unpack path, and -- when packed larger +/// than kv_value_stack_size (256) -- its heap re-read branch. +struct blob_state { + std::string label; + std::vector values; + + SYSLIB_SERIALIZE(blob_state, (label)(values)) +}; +inline bool operator==(const blob_state& a, const blob_state& b) { + return a.label == b.label && a.values == b.values; +} +static_assert(!sysio::kv::is_fixed_serializable_v, + "blob_state must exercise kv::global's packed path"); + +/// A blob whose packed size comfortably exceeds kv_value_stack_size. +blob_state make_big_blob(uint64_t seed) { + blob_state b; + b.label.assign(400, static_cast('a' + (seed % 26))); + for (uint64_t i = 0; i < 20; ++i) b.values.push_back(seed + i); + return b; +} + +// --------------------------------------------------------------------------- +// counting_store -- minimal Store for the generic cached_value semantics +// --------------------------------------------------------------------------- + +/// Satisfies cached_value's Store requirements and records every call. State is static +/// because cached_value owns its Store by value; call counters::reset() to start a case. +struct counting_store { + using value_type = pod_state; + + struct counters { + uint32_t gets = 0; + uint32_t sets = 0; + uint32_t removes = 0; + bool present = false; + pod_state val{}; + sysio::name payer{}; + + static counters& get() { + static counters inst; + return inst; + } + + /// Start a case with an empty store. + static void reset() { get() = counters{}; } + + /// Start a case with \p seeded already stored. + static void seed(const pod_state& seeded) { + reset(); + get().present = true; + get().val = seeded; + } + }; + + bool try_get(pod_state& out) const { + auto& c = counters::get(); + ++c.gets; + if (!c.present) return false; + out = c.val; + return true; + } + + void set(const pod_state& v, sysio::name payer) { + auto& c = counters::get(); + ++c.sets; + c.val = v; + c.payer = payer; + c.present = true; + } + + void remove() { + auto& c = counters::get(); + ++c.removes; + c.present = false; + } +}; + +using counting_cache = sysio::kv::cached_value; + +// --------------------------------------------------------------------------- +// Mocked KV intrinsics -- faithful stand-in for the chain's KV store +// --------------------------------------------------------------------------- + +/// Mirrors apply_context's kv_get / kv_set / kv_erase / kv_contains semantics, including the +/// asymmetry that matters here: reads take a `code` parameter, writes have none and always +/// land on the current receiver. +struct mock_kv { + using row_key = std::tuple; // code, table_id, key + + std::map rows; + uint64_t receiver = 0; + + uint32_t gets = 0; + uint32_t sets = 0; + uint32_t erases = 0; + uint32_t contains = 0; + uint64_t last_payer = 0; + + void reset(uint64_t who) { + rows.clear(); + receiver = who; + gets = sets = erases = contains = 0; + last_payer = 0; + } +}; + +mock_kv& mock_store() { + static mock_kv inst; + return inst; +} + +std::string as_key(const void* key, uint32_t key_size) { + return std::string(static_cast(key), key_size); +} + +/// Install the mocked intrinsics. Idempotent; call once per case after mock_store().reset(). +void install_kv_intrinsics() { + intrinsics::set_intrinsic( + []() -> capi_name { return mock_store().receiver; }); + + // No code parameter: writes always target the current receiver. + intrinsics::set_intrinsic( + [](uint32_t table_id, uint64_t payer, const void* key, uint32_t key_size, + const void* value, uint32_t value_size) -> int64_t { + auto& m = mock_store(); + ++m.sets; + m.last_payer = payer; + auto k = mock_kv::row_key{m.receiver, table_id, as_key(key, key_size)}; + // apply_context::kv_set asserts a valid payer on the CREATE branch; an update may legally + // carry payer 0 (kv::same_payer), which means "keep billing whoever owns the row". Without + // this guard the mock silently accepts a malformed create that the chain would reject. + if (m.rows.find(k) == m.rows.end()) + sysio::check(payer != 0, "must specify a valid account to pay for new record"); + m.rows[k] = std::string(static_cast(value), value_size); + return 0; + }); + + // Returns the FULL stored size even when the caller's buffer is smaller, which is what + // drives kv::global's heap re-read branch. -1 when absent. + intrinsics::set_intrinsic( + [](uint32_t table_id, capi_name code, const void* key, uint32_t key_size, + void* value, uint32_t value_size) -> int32_t { + auto& m = mock_store(); + ++m.gets; + auto itr = m.rows.find(mock_kv::row_key{code, table_id, as_key(key, key_size)}); + if (itr == m.rows.end()) return -1; + const auto sz = static_cast(itr->second.size()); + if (value_size == 0) return static_cast(sz); + const auto copy_size = std::min(value_size, sz); + if (copy_size > 0) std::memcpy(value, itr->second.data(), copy_size); + return static_cast(sz); + }); + + intrinsics::set_intrinsic( + [](uint32_t table_id, const void* key, uint32_t key_size) -> int64_t { + auto& m = mock_store(); + ++m.erases; + auto k = mock_kv::row_key{m.receiver, table_id, as_key(key, key_size)}; + // apply_context::kv_erase asserts the row exists. Issuing an erase for an absent row is a + // real defect -- it aborts the transaction on chain -- so the mock must not absorb it. + sysio::check(m.rows.find(k) != m.rows.end(), "Key not found in `kv_erase`"); + m.rows.erase(k); + return 0; + }); + + intrinsics::set_intrinsic( + [](uint32_t table_id, capi_name code, const void* key, uint32_t key_size) -> int32_t { + auto& m = mock_store(); + ++m.contains; + return m.rows.count(mock_kv::row_key{code, table_id, as_key(key, key_size)}) ? 1 : 0; + }); +} + +constexpr auto test_code = "testacct"_n; +constexpr auto payer_a = "payerone"_n; +constexpr auto payer_b = "payertwo"_n; + +/// Reset the mock store and install the intrinsics for a case. +void begin_kv_case() { + mock_store().reset(test_code.value); + install_kv_intrinsics(); +} + +using pod_global = sysio::kv::global<"cfg"_n, pod_state>; +using pod_cached = sysio::kv::cached_global<"cfg"_n, pod_state>; +using blob_global = sysio::kv::global<"blob"_n, blob_state>; +using blob_cached = sysio::kv::cached_global<"blob"_n, blob_state>; + +} // namespace + +// =========================================================================== +// Group 1 -- generic cached_value semantics over counting_store +// =========================================================================== + +/// The core regression: reading through the cache must not write, not even at destruction. +SYSIO_TEST_BEGIN(cached_read_never_writes) + counting_store::counters::seed(pod_state{7, 3}); + { + counting_cache c; + CHECK_EQUAL(c.exists(), true) + CHECK_EQUAL(c.get(), (pod_state{7, 3})) + CHECK_EQUAL(c.get_or_default(pod_state{99, 99}), (pod_state{7, 3})) + CHECK_EQUAL(c.dirty(), false) + } // destructor runs here -- this is where the old idiom wrote + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) +SYSIO_TEST_END + +/// Reads beyond the first must not touch the store again. +SYSIO_TEST_BEGIN(cached_load_happens_once) + counting_store::counters::seed(pod_state{1, 1}); + { + counting_cache c; + CHECK_EQUAL(c.exists(), true) + const auto after_first = counting_store::counters::get().gets; + CHECK_EQUAL(after_first, 1u) + + (void)c.get(); + (void)c.get_or_default(pod_state{}); + (void)c.exists(); + CHECK_EQUAL(counting_store::counters::get().gets, after_first) + } + CHECK_EQUAL(counting_store::counters::get().sets, 0u) +SYSIO_TEST_END + +/// An absent row reads cleanly and still writes nothing. +SYSIO_TEST_BEGIN(cached_absent_row_reads_without_writing) + counting_store::counters::reset(); + { + counting_cache c; + CHECK_EQUAL(c.exists(), false) + CHECK_EQUAL(c.get_or_default(pod_state{5, 6}), (pod_state{5, 6})) + CHECK_EQUAL(counting_store::counters::get().gets, 1u) + } + CHECK_EQUAL(counting_store::counters::get().sets, 0u) +SYSIO_TEST_END + +/// Many mutations collapse into exactly one write, carrying the final value and last payer. +SYSIO_TEST_BEGIN(cached_modify_defers_single_write) + counting_store::counters::seed(pod_state{10, 0}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter += 5; }); + c.modify(payer_b, [](pod_state& s) { s.counter += 2; s.flags = 42; }); + CHECK_EQUAL(c.dirty(), true) + // Nothing has reached the store yet. + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{17, 42})) + CHECK_EQUAL(counting_store::counters::get().payer, payer_b) +SYSIO_TEST_END + +/// A mutation is visible to later reads on the same handle without re-reading the store. +SYSIO_TEST_BEGIN(cached_reads_see_pending_mutation) + counting_store::counters::seed(pod_state{4, 0}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 88; }); + const auto gets_before = counting_store::counters::get().gets; + CHECK_EQUAL(c.get(), (pod_state{88, 0})) + CHECK_EQUAL(counting_store::counters::get().gets, gets_before) + } + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{88, 0})) +SYSIO_TEST_END + +/// set() creates an absent row, still deferred. +SYSIO_TEST_BEGIN(cached_set_creates_and_defers) + counting_store::counters::reset(); + { + counting_cache c; + c.set(pod_state{3, 9}, payer_a); + CHECK_EQUAL(c.exists(), true) + CHECK_EQUAL(c.get(), (pod_state{3, 9})) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + // set() seeds the cache outright, so it must not need a store read at all. + CHECK_EQUAL(counting_store::counters::get().gets, 0u) + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{3, 9})) +SYSIO_TEST_END + +/// modify_or_create() seeds from the default when absent, then applies the mutation. +SYSIO_TEST_BEGIN(cached_modify_or_create_seeds_default) + counting_store::counters::reset(); + { + counting_cache c; + c.modify_or_create(payer_a, pod_state{100, 1}, [](pod_state& s) { s.counter += 1; }); + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{101, 1})) +SYSIO_TEST_END + +/// modify_or_create() on an existing row ignores the default and mutates what is stored. +SYSIO_TEST_BEGIN(cached_modify_or_create_mutates_existing) + counting_store::counters::seed(pod_state{50, 7}); + { + counting_cache c; + c.modify_or_create(payer_a, pod_state{100, 1}, [](pod_state& s) { s.counter += 1; }); + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{51, 7})) +SYSIO_TEST_END + +/// remove() discards a pending write rather than writing then erasing. +SYSIO_TEST_BEGIN(cached_remove_cancels_pending_write) + counting_store::counters::seed(pod_state{1, 1}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 999; }); + c.remove(); + CHECK_EQUAL(c.exists(), false) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + } + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().removes, 1u) + CHECK_EQUAL(counting_store::counters::get().present, false) +SYSIO_TEST_END + +/// A bare remove() on an untouched handle defers too. +SYSIO_TEST_BEGIN(cached_remove_defers) + counting_store::counters::seed(pod_state{1, 1}); + { + counting_cache c; + c.remove(); + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + } + CHECK_EQUAL(counting_store::counters::get().removes, 1u) +SYSIO_TEST_END + +/// remove() is idempotent. The second call finds the row already absent per the cache -- absent +/// BECAUSE of the first remove() -- and must keep the erase it owes instead of reading that state +/// as "nothing was ever here" and cancelling it, which would leave the stored row untouched while +/// the handle went on reporting it gone. +SYSIO_TEST_BEGIN(cached_remove_is_idempotent) + counting_store::counters::seed(pod_state{4, 5}); + { + counting_cache c; + c.remove(); + c.remove(); + CHECK_EQUAL(c.dirty(), true) + CHECK_EQUAL(c.exists(), false) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + } + CHECK_EQUAL(counting_store::counters::get().removes, 1u) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().present, false) + + // Same rule when the first remove() also had a pending write to discard: the write stays + // cancelled and the erase still survives the second call. + counting_store::counters::seed(pod_state{6, 7}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 999; }); + c.remove(); + c.remove(); + CHECK_EQUAL(c.dirty(), true) + } + CHECK_EQUAL(counting_store::counters::get().removes, 1u) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().present, false) +SYSIO_TEST_END + +/// flush() applies the change once; a second flush and the destructor must not repeat it. +SYSIO_TEST_BEGIN(cached_flush_is_idempotent) + counting_store::counters::seed(pod_state{2, 2}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 20; }); + c.flush(); + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(c.dirty(), false) + c.flush(); + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + } // destructor must not write a third time + CHECK_EQUAL(counting_store::counters::get().sets, 1u) +SYSIO_TEST_END + +/// A mutation after an explicit flush is a fresh debt and gets its own write. +SYSIO_TEST_BEGIN(cached_modify_after_flush_writes_again) + counting_store::counters::seed(pod_state{0, 0}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 1; }); + c.flush(); + c.modify(payer_a, [](pod_state& s) { s.counter = 2; }); + } + CHECK_EQUAL(counting_store::counters::get().sets, 2u) + CHECK_EQUAL(counting_store::counters::get().val, (pod_state{2, 0})) +SYSIO_TEST_END + +// NOTE for every CHECK_ASSERT case below: the native tester implements sysio_assert with longjmp, +// so the handle's destructor -- and therefore its flush() -- does NOT run when the assert fires. +// Each case must assert the store counters itself; without that it verifies only the message text, +// and an implementation that dropped or duplicated the pending change would pass unchanged. + +/// modify() refuses to invent a row -- modify_or_create() is the creating form. +SYSIO_TEST_BEGIN(cached_modify_absent_asserts) + counting_store::counters::reset(); + CHECK_ASSERT("singleton does not exist", ([]() { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 1; }); + })) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + CHECK_EQUAL(counting_store::counters::get().present, false) +SYSIO_TEST_END + +/// get() on an absent row asserts, with the caller's message when supplied. +SYSIO_TEST_BEGIN(cached_get_absent_asserts) + counting_store::counters::reset(); + CHECK_ASSERT("singleton does not exist", ([]() { + counting_cache c; + (void)c.get(); + })) + CHECK_ASSERT("config not initialized", ([]() { + counting_cache c; + (void)c.get("config not initialized"); + })) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) +SYSIO_TEST_END + +/// Mutating a removed handle is a caller bug, not a silent resurrection. +SYSIO_TEST_BEGIN(cached_mutate_after_remove_asserts) + counting_store::counters::seed(pod_state{1, 1}); + CHECK_ASSERT("singleton mutated after remove()", ([]() { + counting_cache c; + c.remove(); + c.modify_or_create(payer_a, pod_state{}, [](pod_state& s) { s.counter = 1; }); + })) + // The rejected call must not have written, and because the longjmp skipped the destructor the + // pending erase was never committed either -- the seeded row is still there, untouched. + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + CHECK_EQUAL(counting_store::counters::get().present, true) + CHECK_EQUAL(counting_store::counters::get().val.counter, 1u) + + // modify() after remove() reports the erase, not the generic "does not exist". + counting_store::counters::seed(pod_state{1, 1}); + CHECK_ASSERT("singleton mutated after remove()", ([]() { + counting_cache c; + c.remove(); + c.modify(payer_a, [](pod_state& s) { s.counter = 2; }); + })) + + // A callback that removes the row through the same handle must not have its erase silently + // converted back into a write of the value it just retired. + counting_store::counters::seed(pod_state{1, 1}); + CHECK_ASSERT("singleton removed from inside a mutation callback", ([]() { + counting_cache c; + c.modify(payer_a, [&c](pod_state& s) { s.counter = 9; c.remove(); }); + })) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) +SYSIO_TEST_END + +/// remove() on a row that is not there owes nothing, so an action that only erases an absent +/// singleton issues no host write and stays legal inside a read-only transaction. Leaving it to +/// the store's own short-circuit would make that legality depend on chain data instead. +SYSIO_TEST_BEGIN(cached_remove_absent_is_noop) + counting_store::counters::reset(); + { + counting_cache c; + c.remove(); + CHECK_EQUAL(c.dirty(), false) + CHECK_EQUAL(c.exists(), false) + } + CHECK_EQUAL(counting_store::counters::get().removes, 0u) + CHECK_EQUAL(counting_store::counters::get().sets, 0u) +SYSIO_TEST_END + +/// same_payer must not overwrite a real payer recorded earlier in the same action. The deferred +/// write coalesces both calls into one store(), and a create billed to payer 0 is rejected on chain. +SYSIO_TEST_BEGIN(cached_same_payer_preserves_recorded_payer) + constexpr sysio::name same_payer{}; + counting_store::counters::reset(); + { + counting_cache c; + c.set(pod_state{1, 1}, payer_a); + c.modify(same_payer, [](pod_state& s) { s.counter = 2; }); + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().payer, payer_a) + + // With no real payer anywhere in the action, same_payer is passed through untouched so the + // host can keep billing whoever owns the row. + counting_store::counters::seed(pod_state{5, 5}); + { + counting_cache c; + c.modify(same_payer, [](pod_state& s) { s.counter = 6; }); + } + CHECK_EQUAL(counting_store::counters::get().payer, same_payer) + + // A later real payer still wins over an earlier one. + counting_store::counters::seed(pod_state{5, 5}); + { + counting_cache c; + c.modify(payer_a, [](pod_state& s) { s.counter = 6; }); + c.modify(payer_b, [](pod_state& s) { s.counter = 7; }); + } + CHECK_EQUAL(counting_store::counters::get().payer, payer_b) + + // set() obeys the same rule as modify(). Uncached, set(v, payer_a) then set(w, same_payer) is + // two writes -- a create billed to payer_a, then an update that legally carries same_payer. The + // cache coalesces them into ONE write, so letting same_payer through would bill a CREATE to + // payer 0, which the host rejects outright. + counting_store::counters::reset(); + { + counting_cache c; + c.set(pod_state{1, 1}, payer_a); + c.set(pod_state{2, 2}, same_payer); + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().payer, payer_a) + CHECK_EQUAL(counting_store::counters::get().val.counter, 2u) +SYSIO_TEST_END + +/// seed_if_absent supplies defaults for reading WITHOUT owing a write -- the property that lets a +/// contract materialize defaults in its constructor and still serve read-only queries. +SYSIO_TEST_BEGIN(cached_seed_if_absent_does_not_dirty) + counting_store::counters::reset(); + { + counting_cache c; + c.seed_if_absent(pod_state{42, 7}); + CHECK_EQUAL(c.dirty(), false) + CHECK_EQUAL(c.exists(), true) + CHECK_EQUAL(c.get().counter, 42u) + } + CHECK_EQUAL(counting_store::counters::get().sets, 0u) + CHECK_EQUAL(counting_store::counters::get().present, false) + + // On an existing row the seed is ignored, and a later mutation writes the STORED value plus the + // change -- not the default. + counting_store::counters::seed(pod_state{5, 5}); + { + counting_cache c; + c.seed_if_absent(pod_state{42, 7}); + CHECK_EQUAL(c.get().counter, 5u) + c.modify(payer_a, [](pod_state& s) { s.counter += 1; }); + } + CHECK_EQUAL(counting_store::counters::get().val.counter, 6u) + + // Seeded defaults do reach storage once something genuinely mutates. + counting_store::counters::reset(); + { + counting_cache c; + c.seed_if_absent(pod_state{42, 7}); + c.modify(payer_a, [](pod_state& s) { s.counter += 1; }); + } + CHECK_EQUAL(counting_store::counters::get().sets, 1u) + CHECK_EQUAL(counting_store::counters::get().val.counter, 43u) + CHECK_EQUAL(counting_store::counters::get().val.flags, 7u) +SYSIO_TEST_END + +/// A reference handed out by get() must stay valid across a remove(): the row is gone but the +/// cached object is not destroyed, so the caller's reference does not dangle. +SYSIO_TEST_BEGIN(cached_reference_survives_remove) + counting_store::counters::seed(pod_state{11, 3}); + { + counting_cache c; + const pod_state& ref = c.get(); + c.remove(); + CHECK_EQUAL(c.exists(), false) + CHECK_EQUAL(ref.counter, 11u) // reading through the reference is still defined + CHECK_EQUAL(ref.flags, 3u) + } + CHECK_EQUAL(counting_store::counters::get().removes, 1u) +SYSIO_TEST_END + +// =========================================================================== +// Group 2 -- kv::cached_global over the real kv::global +// =========================================================================== + +/// The read-only regression, end to end: seed a row, read it through cached_global, let the +/// handle destruct, and require that no kv_set ever happened. This is precisely the sequence +/// that fails on chain today when a contract writes its singleton from the destructor. +SYSIO_TEST_BEGIN(cached_global_read_issues_no_write) + begin_kv_case(); + pod_global(test_code).set(pod_state{11, 22}, payer_a); + const auto sets_after_seed = mock_store().sets; + CHECK_EQUAL(sets_after_seed, 1u) + + { + pod_cached c(test_code); + CHECK_EQUAL(c.exists(), true) + CHECK_EQUAL(c.get(), (pod_state{11, 22})) + CHECK_EQUAL(c.get_or_default(pod_state{}), (pod_state{11, 22})) + } + CHECK_EQUAL(mock_store().sets, sets_after_seed) // no write at destruction + CHECK_EQUAL(mock_store().erases, 0u) + // cached_value loads via try_get, so it never pays for a separate kv_contains probe. + CHECK_EQUAL(mock_store().contains, 0u) +SYSIO_TEST_END + +/// A deferred write lands exactly once and is visible to an independent kv::global handle. +SYSIO_TEST_BEGIN(cached_global_deferred_write_roundtrip) + begin_kv_case(); + pod_global(test_code).set(pod_state{1, 0}, payer_a); + const auto sets_after_seed = mock_store().sets; + + { + pod_cached c(test_code); + c.modify(payer_b, [](pod_state& s) { s.counter = 5; }); + c.modify(payer_b, [](pod_state& s) { s.flags = 6; }); + CHECK_EQUAL(mock_store().sets, sets_after_seed) // still nothing written + } + CHECK_EQUAL(mock_store().sets, sets_after_seed + 1) // exactly one write + CHECK_EQUAL(mock_store().last_payer, payer_b.value) // payer forwarded to kv_set + CHECK_EQUAL(pod_global(test_code).get(), (pod_state{5, 6})) +SYSIO_TEST_END + +/// set() through the cache creates a row that was never there. +/// The same sequence against the real kv::global and the mocked host. This is the end-to-end form +/// of the payer rule: the mock enforces apply_context's "must specify a valid account to pay for new +/// record" on the create branch, so if the coalesced write carried same_payer through, this case +/// would abort inside the intrinsic rather than merely record the wrong payer. +SYSIO_TEST_BEGIN(cached_global_set_then_same_payer_set_creates_row) + constexpr sysio::name same_payer{}; + begin_kv_case(); + { + pod_cached c(test_code); + CHECK_EQUAL(c.exists(), false) + c.set(pod_state{1, 1}, payer_a); // would CREATE the row + c.set(pod_state{2, 2}, same_payer); // legal uncached: the second write is an update + CHECK_EQUAL(mock_store().sets, 0u) + } + CHECK_EQUAL(mock_store().sets, 1u) + CHECK_EQUAL(mock_store().last_payer, payer_a.value) + CHECK_EQUAL(pod_global(test_code).get(), (pod_state{2, 2})) +SYSIO_TEST_END + +SYSIO_TEST_BEGIN(cached_global_set_creates_row) + begin_kv_case(); + { + pod_cached c(test_code); + CHECK_EQUAL(c.exists(), false) + c.set(pod_state{77, 88}, payer_a); + CHECK_EQUAL(mock_store().sets, 0u) + } + CHECK_EQUAL(mock_store().sets, 1u) + CHECK_EQUAL(pod_global(test_code).exists(), true) + CHECK_EQUAL(pod_global(test_code).get(), (pod_state{77, 88})) +SYSIO_TEST_END + +/// remove() through the cache erases the row, deferred like every other change. +SYSIO_TEST_BEGIN(cached_global_remove_erases_row) + begin_kv_case(); + pod_global(test_code).set(pod_state{1, 2}, payer_a); + { + pod_cached c(test_code); + c.remove(); + CHECK_EQUAL(mock_store().erases, 0u) + } + CHECK_EQUAL(mock_store().erases, 1u) + CHECK_EQUAL(pod_global(test_code).exists(), false) +SYSIO_TEST_END + +/// The double-remove case driven through the real kv::global and the mocked host. This is the +/// sharper of the two: it asserts the row is actually gone from storage rather than merely reported +/// gone by the handle, which is exactly what a dropped erase would get wrong. +SYSIO_TEST_BEGIN(cached_global_double_remove_erases_row) + begin_kv_case(); + pod_global(test_code).set(pod_state{3, 4}, payer_a); + { + pod_cached c(test_code); + c.remove(); + c.remove(); + CHECK_EQUAL(mock_store().erases, 0u) + } + CHECK_EQUAL(mock_store().erases, 1u) + CHECK_EQUAL(pod_global(test_code).exists(), false) +SYSIO_TEST_END + +/// The packed (non-fixed-serializable) path, including a payload big enough to force +/// kv::global's heap re-read. Reads must still issue no write. +SYSIO_TEST_BEGIN(cached_global_blob_roundtrip) + begin_kv_case(); + const auto seeded = make_big_blob(1); + CHECK_EQUAL(sysio::pack_size(seeded) > sysio::kv::kv_value_stack_size, true) + + blob_global(test_code).set(seeded, payer_a); + const auto sets_after_seed = mock_store().sets; + + { + blob_cached c(test_code); + CHECK_EQUAL(c.get(), seeded) + const auto gets_after_load = mock_store().gets; + (void)c.get(); // cached: no further store reads + CHECK_EQUAL(mock_store().gets, gets_after_load) + } + CHECK_EQUAL(mock_store().sets, sets_after_seed) // read-only: still no write + + { + blob_cached c(test_code); + c.modify(payer_b, [](blob_state& b) { b.values.push_back(4242); }); + } + CHECK_EQUAL(mock_store().sets, sets_after_seed + 1) + auto expected = seeded; + expected.values.push_back(4242); + CHECK_EQUAL(blob_global(test_code).get(), expected) +SYSIO_TEST_END + +/// modify_or_create() creates through the real store when the row is absent. +SYSIO_TEST_BEGIN(cached_global_modify_or_create_creates_row) + begin_kv_case(); + { + pod_cached c(test_code); + c.modify_or_create(payer_a, pod_state{9, 9}, [](pod_state& s) { s.counter += 1; }); + CHECK_EQUAL(mock_store().sets, 0u) + } + CHECK_EQUAL(mock_store().sets, 1u) + CHECK_EQUAL(pod_global(test_code).get(), (pod_state{10, 9})) +SYSIO_TEST_END + +/// Two sequential handles behave like two sequential actions: the first flush is visible to +/// the second handle. +SYSIO_TEST_BEGIN(cached_global_sequential_handles_observe_flush) + begin_kv_case(); + pod_global(test_code).set(pod_state{0, 0}, payer_a); + { + pod_cached c(test_code); + c.modify(payer_a, [](pod_state& s) { s.counter = 1; }); + } + { + pod_cached c(test_code); + CHECK_EQUAL(c.get(), (pod_state{1, 0})) + c.modify(payer_a, [](pod_state& s) { s.counter += 1; }); + } + CHECK_EQUAL(pod_global(test_code).get(), (pod_state{2, 0})) +SYSIO_TEST_END + +// =========================================================================== +// Group 3 -- Store conformance +// =========================================================================== + +/// A stored row whose size does not match the fixed-serializable payload must be rejected outright. +/// kv_get fills min(buffer, stored) bytes but reports the full stored size, so copying sizeof(T) out +/// of a short row splices whatever the contract's linear memory held into the payload. Deterministic +/// -- every node builds the same wrong value -- but it reads as authoritative stored data, so trap. +SYSIO_TEST_BEGIN(global_rejects_wrong_sized_row) + begin_kv_case(); + { + pod_global g{test_code}; + g.set(pod_state{1, 2}, payer_a); + } + // Shorten the stored row, as an incompatible earlier version of the payload would have left it. + for (auto& row : mock_store().rows) row.second.resize(row.second.size() - 1); + + CHECK_ASSERT("kv::global: stored value size does not match the fixed-serializable payload", ([]() { + pod_global g{test_code}; + pod_state out; + (void)g.try_get(out); + })) + + // The cached wrapper loads through the same path, so it inherits the rejection. + CHECK_ASSERT("kv::global: stored value size does not match the fixed-serializable payload", ([]() { + pod_cached c{test_code}; + (void)c.exists(); + })) +SYSIO_TEST_END + +static_assert(std::is_same_v, + "cached_global must expose the store's payload type"); +static_assert(!std::is_copy_constructible_v, + "cached_value must not be copyable -- two handles would own conflicting writes"); + +// The scoped sysio::cached_kv_singleton is exercised by kv_cached_contract, driven from +// tests/integration/kv_cached_tests.cpp -- NOT by kv_singleton_tests, which predates this feature +// and drives sysio::singleton. +// +// It is not covered here because kv_singleton is backed by kv_multi_index, whose find() needs the +// kv_it_* iterator intrinsics on top of the four this file mocks. (Including the header natively is +// no longer the obstacle: kv_singleton.hpp used to pull in contracts/sysio/system.hpp, whose +// is_feature_activated declaration collides with the C-API one the native tester declares, and that +// include has been removed as unused.) Extending the mock with a positioned iterator would bring +// the scoped alias into this natively-run suite, which matters because the integration suite is +// gated behind ENABLE_INTEGRATION_TESTS and does not run in CI. + +int main(int argc, char* argv[]) { + bool verbose = false; + if (argc >= 2 && std::strcmp(argv[1], "-v") == 0) { + verbose = true; + } + silence_output(!verbose); + + SYSIO_TEST(cached_read_never_writes) + SYSIO_TEST(cached_load_happens_once) + SYSIO_TEST(cached_absent_row_reads_without_writing) + SYSIO_TEST(cached_modify_defers_single_write) + SYSIO_TEST(cached_reads_see_pending_mutation) + SYSIO_TEST(cached_set_creates_and_defers) + SYSIO_TEST(cached_modify_or_create_seeds_default) + SYSIO_TEST(cached_modify_or_create_mutates_existing) + SYSIO_TEST(cached_remove_cancels_pending_write) + SYSIO_TEST(cached_remove_defers) + SYSIO_TEST(cached_remove_is_idempotent) + SYSIO_TEST(cached_flush_is_idempotent) + SYSIO_TEST(cached_modify_after_flush_writes_again) + SYSIO_TEST(cached_modify_absent_asserts) + SYSIO_TEST(cached_get_absent_asserts) + SYSIO_TEST(cached_mutate_after_remove_asserts) + SYSIO_TEST(cached_remove_absent_is_noop) + SYSIO_TEST(cached_same_payer_preserves_recorded_payer) + SYSIO_TEST(cached_seed_if_absent_does_not_dirty) + SYSIO_TEST(cached_reference_survives_remove) + + SYSIO_TEST(cached_global_read_issues_no_write) + SYSIO_TEST(cached_global_deferred_write_roundtrip) + SYSIO_TEST(cached_global_set_creates_row) + SYSIO_TEST(cached_global_set_then_same_payer_set_creates_row) + SYSIO_TEST(cached_global_remove_erases_row) + SYSIO_TEST(cached_global_double_remove_erases_row) + SYSIO_TEST(cached_global_blob_roundtrip) + SYSIO_TEST(cached_global_modify_or_create_creates_row) + SYSIO_TEST(cached_global_sequential_handles_observe_flush) + SYSIO_TEST(global_rejects_wrong_sized_row) + + return has_failed(); +} diff --git a/tests/unit/test_contracts/CMakeLists.txt b/tests/unit/test_contracts/CMakeLists.txt index f39633d74..56375fddc 100644 --- a/tests/unit/test_contracts/CMakeLists.txt +++ b/tests/unit/test_contracts/CMakeLists.txt @@ -18,6 +18,7 @@ add_contract(mi_scope_tests mi_scope_tests multi_index_scope_tests.cpp) add_contract(kv_indexed_table_tests kv_indexed_table_tests kv_indexed_table_tests.cpp) add_contract(kv_singleton_tests kv_singleton_tests kv_singleton_tests.cpp) add_contract(kv_global_tests kv_global_tests kv_global_tests.cpp) +add_contract(kv_cached_contract kv_cached_contract kv_cached_contract.cpp) add_contract(kv_scoped_table_tests kv_scoped_table_tests kv_scoped_table_tests.cpp) add_contract(hash_id_tests hash_id_tests hash_id_tests.cpp) add_contract(capi_tests capi_tests capi/capi.c capi/action.c capi/chain.c capi/crypto.c capi/db.c capi/permission.c diff --git a/tests/unit/test_contracts/kv_cached_contract.cpp b/tests/unit/test_contracts/kv_cached_contract.cpp new file mode 100644 index 000000000..d3221a572 --- /dev/null +++ b/tests/unit/test_contracts/kv_cached_contract.cpp @@ -0,0 +1,136 @@ +/** + * @file + * @copyright defined in sysio.cdt/LICENSE.txt + * + * Contract-side coverage for kv::cached_global and sysio::cached_kv_singleton. + * + * Both singletons are held as CONTRACT MEMBERS, which is the shape that matters: the + * generated dispatcher instantiates the contract as a temporary and destroys it the moment + * the action returns, so a member that unconditionally writes itself back at destruction + * makes EVERY action -- including a pure query -- issue a kv_set and fail inside a read-only + * transaction. The cached types only write when an action actually mutated something, so + * `roread` below is legal read-only while `bump` is correctly still rejected. + * + * Driven by tests/integration/kv_cached_tests.cpp. + */ + +#include +#include +#include + +using namespace sysio; + +class [[sysio::contract("kv_cached_contract")]] kv_cached_contract : public contract { +public: + // No default member initializers below. An NSDMI on a NESTED class is parsed in the + // ENCLOSING class's complete-class context, so while kv_cached_contract is still being + // defined the payload's implicit default constructor is not yet known and the + // is_default_constructible_v assertions inside kv::global / kv_multi_index both fail. + + /// Unscoped payload, fixed-serializable (kv::global's zero-copy path). + struct [[sysio::table("cachcfg")]] cfg { + uint64_t rate; + uint64_t flags; + SYSLIB_SERIALIZE(cfg, (rate)(flags)) + }; + + /// Scoped payload, variable length (kv_singleton's pack/unpack path). + struct [[sysio::table("cachsng")]] scoped_cfg { + uint64_t version; + std::string label; + SYSLIB_SERIALIZE(scoped_cfg, (version)(label)) + }; + + using cfg_cached = kv::cached_global<"cachcfg"_n, cfg>; + using sng_cached = cached_kv_singleton<"cachsng"_n, scoped_cfg>; + + kv_cached_contract(name s, name code, datastream ds) + : contract(s, code, ds) + , _cfg(s) + , _sng(s, s.value) {} + + /// Create both rows. Deferred: the writes land when the members destruct. + [[sysio::action]] + void seed() { + _cfg.set({42, 7}, get_self()); + _sng.set({1, "hello"}, get_self()); + } + + /// Pure reads. Issues no kv_set at all, so this action is legal inside a read-only + /// transaction -- the whole point of the cached types. + [[sysio::action]] + void roread() { + check(_cfg.exists(), "roread: cfg row missing"); + check(_cfg.get().rate == 42, "roread: cfg rate"); + check(_cfg.get().flags == 7, "roread: cfg flags"); + + check(_sng.exists(), "roread: sng row missing"); + check(_sng.get().version == 1, "roread: sng version"); + check(_sng.get().label == "hello", "roread: sng label"); + } + + /// Reads that miss are still reads: no write, so this stays read-only legal even though + /// the rows are absent. + [[sysio::action]] + void roabsent() { + check(!_cfg.exists(), "roabsent: cfg row should be absent"); + check(_cfg.get_or_default({99, 99}).rate == 99, "roabsent: cfg default"); + check(_sng.get_or_default({7, "def"}).label == "def", "roabsent: sng default"); + } + + /// Mutates both singletons. Must still be REJECTED inside a read-only transaction. + [[sysio::action]] + void bump() { + _cfg.modify(get_self(), [](cfg& c) { c.rate += 1; }); + _sng.modify(get_self(), [](scoped_cfg& s) { s.version += 1; }); + } + + /// Confirms a previous bump's deferred writes actually reached the store. + [[sysio::action]] + void chkbump() { + check(_cfg.get().rate == 43, "chkbump: cfg rate did not persist"); + check(_cfg.get().flags == 7, "chkbump: cfg flags changed unexpectedly"); + check(_sng.get().version == 2, "chkbump: sng version did not persist"); + check(_sng.get().label == "hello", "chkbump: sng label changed unexpectedly"); + } + + /// Repeated mutations inside one action must collapse into a single stored result. + [[sysio::action]] + void multibump() { + for (int i = 0; i < 5; ++i) { + _cfg.modify(get_self(), [](cfg& c) { c.rate += 10; }); + } + } + + /// Confirms multibump stored ONE result rather than five: 43 + 5*10. + [[sysio::action]] + void chkmulti() { + check(_cfg.get().rate == 93, "chkmulti: cfg rate did not persist"); + } + + /// modify_or_create() creates from the default when the row is absent. + [[sysio::action]] + void createnew() { + _cfg.modify_or_create(get_self(), {100, 1}, [](cfg& c) { c.rate += 1; }); + } + + /// Confirms the row createnew created actually persisted. + [[sysio::action]] + void chkcreate() { + check(_cfg.get().rate == 101, "chkcreate: cfg rate did not persist"); + check(_cfg.get().flags == 1, "chkcreate: cfg flags did not persist"); + } + + /// Deferred erase. + [[sysio::action]] + void rmall() { + _cfg.remove(); + _sng.remove(); + check(!_cfg.exists(), "rmall: cfg should read absent immediately"); + check(!_sng.exists(), "rmall: sng should read absent immediately"); + } + +private: + cfg_cached _cfg; + sng_cached _sng; +};