From c36c5fb252cff5c0c5b79ef12f6584df702e6ff5 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 20:14:53 -0400 Subject: [PATCH 01/11] feat(rentals): non-custodial renter-as-owner lease primitives Replace the custodial holders/move machinery with a native lock + title model. During a lease the renter becomes the real AtomicAssets owner, the lister's reclaim right is parked in a `leases` row, and the asset is locked from transfer/burn/offer-out while that row exists. A permissionless `reclaim` force-returns the asset to the title_owner at expiry, so neither party can strand it. - remove the move action, holders table and logmove - add the leases table + check_not_leased guards on internal_transfer (chokepoint, covers transfer + acceptoffer), burnasset and createoffer; setassetdata is deliberately left unguarded (collection-auth gated) - add pretitle/leasestart/leaseextend/delpretitle/reclaim, a config'd rental_market authority gate (setrentmkt, defaults to atomicmarket) and loglock/logreclaim log actions; offers are cleared on lease-start/reclaim - replace the custodial renting characterization tests with a non-custodial lease/lock suite Experimental branch for smart-contract review. --- include/atomicassets-interface.hpp | 25 +- include/atomicassets.hpp | 102 +++- src/atomicassets.cpp | 444 +++++++++++---- tests/Admin Actions/init.test.js | 6 +- tests/Asset Actions/move.test.js | 504 ------------------ .../Asset Actions/renting-invariants.test.js | 459 +++++++++++----- .../burnasset.test.js | 44 -- tests/Transfer-Offer Actions/transfer.test.js | 207 ------- 8 files changed, 745 insertions(+), 1046 deletions(-) delete mode 100644 tests/Asset Actions/move.test.js diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index ab43ac2..6982cc3 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -148,17 +148,21 @@ namespace atomicassets { typedef multi_index assets_t; - struct holders_s { + struct leases_s { uint64_t asset_id; - name holder; - name owner; - - uint64_t primary_key() const { return asset_id; }; - uint64_t by_holder() const { return holder.value; }; + name title_owner; + name renter; + uint32_t rental_end; + name market; + + uint64_t primary_key() const { return asset_id; }; + uint64_t by_title_owner() const { return title_owner.value; }; + uint64_t by_rental_end() const { return (uint64_t) rental_end; }; }; - typedef multi_index >> - holders_t; + typedef multi_index >, + indexed_by>> + leases_t; struct offers_s { @@ -197,6 +201,7 @@ namespace atomicassets { uint64_t offer_counter = 1; vector collection_format = {}; vector supported_tokens = {}; + name rental_market = name("atomicmarket"); }; typedef singleton config_t; @@ -228,6 +233,6 @@ namespace atomicassets { template_mutables_t get_template_mutables(name collection_name) {return template_mutables_t(get_self(), collection_name.value);} assets_t get_assets(name owner) {return assets_t(get_self(), owner.value);} - holders_t get_holders() {return holders_t(get_self(), get_self().value);} + leases_t get_leases() {return leases_t(get_self(), get_self().value);} }; \ No newline at end of file diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index c8bf373..d2fcb65 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -34,14 +34,39 @@ CONTRACT atomicassets : public contract { string memo ); - ACTION move( - name owner, - name from, - name to, - vector asset_ids, + ACTION setrentmkt( + name rental_market + ); + + ACTION pretitle( + name title_owner, + name market, + uint64_t asset_id + ); + + ACTION leasestart( + name market, + name title_owner, + name renter, + uint64_t asset_id, + uint32_t rental_end, string memo ); + ACTION leaseextend( + name market, + uint64_t asset_id, + uint32_t rental_end + ); + + ACTION delpretitle( + uint64_t asset_id + ); + + ACTION reclaim( + uint64_t asset_id + ); + ACTION createcol( name author, name collection_name, @@ -260,13 +285,20 @@ CONTRACT atomicassets : public contract { string memo ); - ACTION logmove( + ACTION loglock( name collection_name, - name owner, - name from, - name to, - vector asset_ids, - string memo + uint64_t asset_id, + name title_owner, + name renter, + uint32_t rental_end, + name market + ); + + ACTION logreclaim( + name collection_name, + uint64_t asset_id, + name title_owner, + name renter ); ACTION lognewoffer( @@ -438,17 +470,29 @@ CONTRACT atomicassets : public contract { typedef multi_index assets_t; - TABLE holders_s { + // Non-custodial rental "title" / lock record. The existence of a row for an + // asset_id means the asset is LOCKED (no transfer/burn/offer-out/sale). Two + // states: + // pretitle sentinel: renter == name(""), rental_end == 0 — the lister has + // pre-locked the asset and consented to `market`; owner is still the + // lister. + // active lease: renter != name(""), rental_end > 0 — the renter is the + // real AtomicAssets owner; title_owner holds the reclaim right. + TABLE leases_s { uint64_t asset_id; - name holder; - name owner; - - uint64_t primary_key() const { return asset_id; }; - uint64_t by_holder() const { return holder.value; }; + name title_owner; // lister; reclaim returns the asset here + name renter; // current AA owner during the lease + uint32_t rental_end; // sec_since_epoch; 0 for a pretitle sentinel + name market; // rental market that opened/manages the lease + + uint64_t primary_key() const { return asset_id; }; + uint64_t by_title_owner() const { return title_owner.value; }; + uint64_t by_rental_end() const { return (uint64_t) rental_end; }; }; - typedef multi_index >> - holders_t; + typedef multi_index >, + indexed_by>> + leases_t; TABLE offers_s { @@ -487,6 +531,12 @@ CONTRACT atomicassets : public contract { uint64_t offer_counter = 1; vector collection_format = {}; vector supported_tokens = {}; + // The single account authorized to open/manage non-custodial rental + // leases (pretitle/leasestart/leaseextend). Defaults to the AtomicMarket + // contract; set to name("") via setrentmkt to disable leasing. + // NOTE: appending this field requires re-initialising the config + // singleton on an existing deployment (the stored blob predates it). + name rental_market = name("atomicmarket"); }; typedef singleton config_t; @@ -518,7 +568,7 @@ CONTRACT atomicassets : public contract { template_mutables_t get_template_mutables(name collection_name) {return template_mutables_t(get_self(), collection_name.value);} assets_t get_assets(name owner) {return assets_t(get_self(), owner.value);} - holders_t get_holders() {return holders_t(get_self(), get_self().value);} + leases_t get_leases() {return leases_t(get_self(), get_self().value);} /* ************************** @@ -542,7 +592,8 @@ CONTRACT atomicassets : public contract { name to, vector asset_ids, string memo, - name scope_payer + name scope_payer, + bool enforce_lock = true ); void internal_decrease_balance( @@ -560,6 +611,13 @@ CONTRACT atomicassets : public contract { name & collection_name ); + // Reverts if the asset has a live lease/title record (i.e. is rental-locked). + void check_not_leased(uint64_t asset_id); + + // Erases any offers created by `account` whose sender_asset_ids reference + // `asset_id` (offers are AtomicAssets' approval mechanism). + void clear_offers_for_asset(name account, uint64_t asset_id); + void notify_collection_accounts( name collection_name ); diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index 5ffe36b..17f2033 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -86,106 +86,273 @@ ACTION atomicassets::transfer( } /** -* Moves one or more assets to another account -* @required_auth of the true owner of the asset -* Cannot have notifications for the from & to, exploitable +* Sets the single account authorized to open/manage non-custodial rental leases +* (pretitle / leasestart / leaseextend). name("") disables leasing entirely. +* @required_auth The contract itself */ -ACTION atomicassets::move( - name owner, - name from, - name to, - vector asset_ids, - string memo +ACTION atomicassets::setrentmkt(name rental_market) { + require_auth(get_self()); + + check(rental_market == name("") || is_account(rental_market), + "rental_market account does not exist"); + + auto config = get_config(); + config_s current_config = config.get(); + current_config.rental_market = rental_market; + config.set(current_config, get_self()); +} + + +/** +* Pre-locks an asset for non-custodial rental and records the lister's consent +* to `market` BEFORE any renter exists. Owner stays the lister; the asset is +* immediately locked (a lease row exists). This is the lister-signed half of the +* hybrid lease-start handshake — the market can later activate a lease only on +* assets that carry such a row, capping the market's blast radius. +* @required_auth title_owner +*/ +ACTION atomicassets::pretitle( + name title_owner, + name market, + uint64_t asset_id ) { - require_auth(owner); - require_recipient(owner); + require_auth(title_owner); - check(is_account(from), "from account does not exist"); - check(is_account(to), "to account does not exist"); + auto config = get_config(); + config_s current_config = config.get(); + check(current_config.rental_market != name("") && market == current_config.rental_market, + "market is not the configured rental market"); - check(from != to, "from & to fields cannot be the same"); + assets_t owner_assets = get_assets(title_owner); + auto asset_itr = owner_assets.require_find(asset_id, + "title_owner does not own this asset"); - check(asset_ids.size() != 0, "asset_ids needs to contain at least one id"); - check(memo.length() <= 256, "A move memo can only be 256 characters max"); + // A non-transferable asset can never be leased out. + if (asset_itr->template_id >= 0) { + templates_t collection_templates = get_templates(asset_itr->collection_name); + auto template_itr = collection_templates.find(asset_itr->template_id); + check(template_itr->transferable, "The asset is not transferable"); + } - vector asset_ids_copy = asset_ids; - std::sort(asset_ids_copy.begin(), asset_ids_copy.end()); - check(std::adjacent_find(asset_ids_copy.begin(), asset_ids_copy.end()) == asset_ids_copy.end(), - "Can't move the same asset multiple times"); + leases_t leases = get_leases(); + check(leases.find(asset_id) == leases.end(), "Asset is already leased or pre-titled"); - assets_t owner_assets = get_assets(owner); - holders_t holders = get_holders(); + leases.emplace(title_owner, [&](auto &_lease) { + _lease.asset_id = asset_id; + _lease.title_owner = title_owner; + _lease.renter = name(""); + _lease.rental_end = 0; + _lease.market = market; + }); - map > collection_to_assets_moved = {}; + action( + permission_level{get_self(), name("active")}, + get_self(), + name("loglock"), + make_tuple(asset_itr->collection_name, asset_id, title_owner, name(""), (uint32_t) 0, market) + ).send(); +} - for (uint64_t & asset_id : asset_ids) { - auto asset_itr = owner_assets.find(asset_id); - if (asset_itr == owner_assets.end()){ - check(false, - ("Owner doesn't own at least one of the provided assets (ID: " + to_string(asset_id) + ")").c_str()); - } - - //Existence doesn't have to be checked because this always has to exist - if (asset_itr->template_id >= 0) { - templates_t collection_templates = get_templates(asset_itr->collection_name); +/** +* Opens a non-custodial rental lease: makes `renter` the real AtomicAssets owner +* of the asset and parks the lister's reclaim right in the lease record, with NO +* unlocked window (the lease row is written before the ownership flip). May +* activate an existing pretitle sentinel or open a lease directly. +* @required_auth market (must be the configured rental_market) +*/ +ACTION atomicassets::leasestart( + name market, + name title_owner, + name renter, + uint64_t asset_id, + uint32_t rental_end, + string memo +) { + require_auth(market); - auto template_itr = collection_templates.find(asset_itr->template_id); - if (!template_itr->transferable){ - check(false, - ("At least one asset isn't transferable (ID: " + to_string(asset_id) + ")").c_str()); - } - } + auto config = get_config(); + config_s current_config = config.get(); + check(current_config.rental_market != name("") && market == current_config.rental_market, + "market is not the configured rental market"); - auto holders_itr = holders.find(asset_id); - if (holders_itr == holders.end()){ - if (from != owner){ - check(false, - ("Only the owner can move this asset (ID: " + to_string(asset_id) + ")").c_str()); - } - - // Emplaces new holder - holders.emplace(owner, [&](auto &_holders_row){ - _holders_row.asset_id = asset_id; - _holders_row.holder = to; - _holders_row.owner = owner; - }); - } + check(is_account(renter), "renter account does not exist"); + check(renter != title_owner, "renter and title_owner cannot be the same"); - if (holders_itr != holders.end()){ - if (holders_itr->holder != from){ - check(false, - ("At least one asset invalidates the 'from:holder' constraint (ID: " + to_string(asset_id) + ")").c_str()); - } + uint32_t now = eosio::current_time_point().sec_since_epoch(); + check(rental_end > now, "rental_end must be in the future"); - // Deletes row if returning to owner - if (to == owner){ - holders.erase(holders_itr); - } else { // Modifies row to move holdership to the new "to" wallet - holders.modify(holders_itr, owner, [&](auto &_holders_row){ - _holders_row.holder = to; - }); - } - } + // Reject an already-active lease up front (the title_owner no longer owns the + // asset in that state, so the ownership check below would mask the cause). + leases_t leases = get_leases(); + auto lease_itr = leases.find(asset_id); + check(lease_itr == leases.end() || lease_itr->renter == name(""), + "Asset is already leased"); - //This is needed for sending notifications later - if (collection_to_assets_moved.find(asset_itr->collection_name) != - collection_to_assets_moved.end()) { - collection_to_assets_moved[asset_itr->collection_name].push_back(asset_id); - } else { - collection_to_assets_moved[asset_itr->collection_name] = {asset_id}; - } + assets_t owner_assets = get_assets(title_owner); + auto asset_itr = owner_assets.require_find(asset_id, + "title_owner does not own this asset"); + name collection_name = asset_itr->collection_name; + + // A non-transferable asset can never be leased out (fail early with a clear + // message; internal_transfer would otherwise reject it after the row write). + if (asset_itr->template_id >= 0) { + templates_t collection_templates = get_templates(asset_itr->collection_name); + auto template_itr = collection_templates.find(asset_itr->template_id); + check(template_itr->transferable, "The asset is not transferable"); } - // Sending notifications - for (const auto&[collection, assets_moved] : collection_to_assets_moved) { - action( - permission_level{get_self(), name("active")}, - get_self(), - name("logmove"), - make_tuple(collection, owner, from, to, assets_moved, memo) - ).send(); + if (lease_itr == leases.end()) { + // Direct lease-start (no prior pretitle). Write the lock row FIRST. + leases.emplace(market, [&](auto &_lease) { + _lease.asset_id = asset_id; + _lease.title_owner = title_owner; + _lease.renter = renter; + _lease.rental_end = rental_end; + _lease.market = market; + }); + } else { + // Activating a pretitle sentinel. + check(lease_itr->title_owner == title_owner, "lease title_owner mismatch"); + check(lease_itr->renter == name(""), "Asset is already leased"); + leases.modify(lease_itr, market, [&](auto &_lease) { + _lease.renter = renter; + _lease.rental_end = rental_end; + _lease.market = market; + }); } + + // Close the stale-offer escape: drop any offers the lister created that + // reference this asset before it changes hands. + clear_offers_for_asset(title_owner, asset_id); + + // Flip ownership lister -> renter under the contract's own authority. The + // lock is already in force, so this is the privileged (enforce_lock=false) + // path. The contract pays any transient scope RAM. + internal_transfer(title_owner, renter, vector{asset_id}, memo, get_self(), false); + + action( + permission_level{get_self(), name("active")}, + get_self(), + name("loglock"), + make_tuple(collection_name, asset_id, title_owner, renter, rental_end, market) + ).send(); +} + + +/** +* Extends an active lease's end time. Does not change ownership. +* @required_auth market (must be the configured rental_market) +*/ +ACTION atomicassets::leaseextend( + name market, + uint64_t asset_id, + uint32_t rental_end +) { + require_auth(market); + + auto config = get_config(); + config_s current_config = config.get(); + check(current_config.rental_market != name("") && market == current_config.rental_market, + "market is not the configured rental market"); + + leases_t leases = get_leases(); + auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); + check(lease_itr->renter != name(""), "Asset is pre-titled but not leased"); + check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); + + name title_owner = lease_itr->title_owner; + name renter = lease_itr->renter; + + assets_t renter_assets = get_assets(renter); + auto asset_itr = renter_assets.require_find(asset_id, "renter no longer owns the asset"); + name collection_name = asset_itr->collection_name; + + leases.modify(lease_itr, market, [&](auto &_lease) { + _lease.rental_end = rental_end; + }); + + action( + permission_level{get_self(), name("active")}, + get_self(), + name("loglock"), + make_tuple(collection_name, asset_id, title_owner, renter, rental_end, market) + ).send(); +} + + +/** +* Clears a pretitle sentinel (a pre-locked asset that was never leased), +* unlocking it. Only valid before a lease is opened. +* @required_auth title_owner +*/ +ACTION atomicassets::delpretitle( + uint64_t asset_id +) { + leases_t leases = get_leases(); + auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); + check(lease_itr->renter == name(""), + "Asset is actively leased; use reclaim after expiry"); + + require_auth(lease_itr->title_owner); + + name title_owner = lease_itr->title_owner; + assets_t owner_assets = get_assets(title_owner); + auto asset_itr = owner_assets.require_find(asset_id, "title_owner does not own this asset"); + name collection_name = asset_itr->collection_name; + + leases.erase(lease_itr); + + action( + permission_level{get_self(), name("active")}, + get_self(), + name("logreclaim"), + make_tuple(collection_name, asset_id, title_owner, name("")) + ).send(); +} + + +/** +* Permissionless reclaim of an expired lease: returns ownership from the renter +* to the title_owner and clears the lock. Callable by anyone once the lease has +* expired; the renter's signature is never required (the move runs under the +* contract's own authority). This is the guaranteed revert the whole model rests +* on. +* @required_auth none (permissionless) +*/ +ACTION atomicassets::reclaim( + uint64_t asset_id +) { + leases_t leases = get_leases(); + auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); + check(lease_itr->renter != name(""), + "Asset is pre-titled but not leased; use delpretitle"); + + uint32_t now = eosio::current_time_point().sec_since_epoch(); + check(now >= lease_itr->rental_end, "Lease has not expired yet"); + + name title_owner = lease_itr->title_owner; + name renter = lease_itr->renter; + + assets_t renter_assets = get_assets(renter); + auto asset_itr = renter_assets.require_find(asset_id, "renter no longer owns the asset"); + name collection_name = asset_itr->collection_name; + + // Close the stale-offer escape on the renter side before the asset moves. + clear_offers_for_asset(renter, asset_id); + + // Erase the lock, then move the asset back under the contract's own + // authority (enforce_lock=false). The contract pays any transient scope RAM + // so reclaim never needs the title_owner's signature. + leases.erase(lease_itr); + internal_transfer(renter, title_owner, vector{asset_id}, "lease reclaim", get_self(), false); + + action( + permission_level{get_self(), name("active")}, + get_self(), + name("logreclaim"), + make_tuple(collection_name, asset_id, title_owner, renter) + ).send(); } /** @@ -1196,6 +1363,10 @@ ACTION atomicassets::burnasset( ) { require_auth(asset_owner); + // A rental-locked asset cannot be burned (that would destroy the lister's + // reclaim right). + check_not_leased(asset_id); + assets_t owner_assets = get_assets(asset_owner); auto asset_itr = owner_assets.require_find(asset_id, "No asset with this id exists for this owner"); @@ -1207,14 +1378,6 @@ ACTION atomicassets::burnasset( check(template_itr->burnable, "The asset is not burnable"); }; - holders_t holders = get_holders(); - - // Checks to see if the asset has been rented out & erases the "holdership" - auto holders_itr = holders.find(asset_id); - if (holders_itr != holders.end()){ - holders.erase(holders_itr); - } - if (asset_itr->backed_tokens.size() != 0) { auto balances = get_balances(); auto balance_itr = balances.find(asset_owner.value); @@ -1321,10 +1484,14 @@ ACTION atomicassets::createoffer( for (uint64_t asset_id : sender_asset_ids) { auto asset_itr = sender_assets.find(asset_id); if (asset_itr == sender_assets.end()){ - check(false, + check(false, ("Offer sender doesn't own at least one of the provided assets (ID: " + to_string(asset_id) + ")").c_str()); } + // A renter is the real owner of a leased asset, so get_assets(sender) + // exposes it here. Block offering a rental-locked asset out. + check_not_leased(asset_id); + if (asset_itr->template_id >= 0) { templates_t collection_templates = get_templates(asset_itr->collection_name); @@ -1559,16 +1726,35 @@ ACTION atomicassets::logtransfer( notify_collection_accounts(collection_name); } -ACTION atomicassets::logmove( +ACTION atomicassets::loglock( name collection_name, - name owner, - name from, - name to, - vector asset_ids, - string memo + uint64_t asset_id, + name title_owner, + name renter, + uint32_t rental_end, + name market ) { require_auth(get_self()); + require_recipient(title_owner); + if (renter != name("")) { + require_recipient(renter); + } + notify_collection_accounts(collection_name); +} + +ACTION atomicassets::logreclaim( + name collection_name, + uint64_t asset_id, + name title_owner, + name renter +) { + require_auth(get_self()); + + require_recipient(title_owner); + if (renter != name("")) { + require_recipient(renter); + } notify_collection_accounts(collection_name); } @@ -1786,7 +1972,8 @@ void atomicassets::internal_transfer( name to, vector asset_ids, string memo, - name scope_payer + name scope_payer, + bool enforce_lock ) { check(is_account(to), "to account does not exist"); @@ -1803,17 +1990,20 @@ void atomicassets::internal_transfer( assets_t from_assets = get_assets(from); assets_t to_assets = get_assets(to); - holders_t holders = get_holders(); map > collection_to_assets_transferred = {}; for (uint64_t asset_id : asset_ids) { auto asset_itr = from_assets.find(asset_id); if (asset_itr == from_assets.end()){ - check(false, + check(false, ("Sender doesn't own at least one of the provided assets (ID: " + to_string(asset_id) + ")").c_str()); } + // Rental lock: a leased asset can only be moved by the privileged + // lease-start / reclaim paths (which pass enforce_lock = false). + if (enforce_lock) check_not_leased(asset_id); + //Existence doesn't have to be checked because this always has to exist if (asset_itr->template_id >= 0) { templates_t collection_templates = get_templates(asset_itr->collection_name); @@ -1825,19 +2015,6 @@ void atomicassets::internal_transfer( } } - auto holders_itr = holders.find(asset_id); - if (holders_itr != holders.end()){ - - // Deletes row if transfering to holder - if (to == holders_itr->holder){ - holders.erase(holders_itr); - } else { // Modifies row to move ownership to the new "to" wallet - holders.modify(holders_itr, from, [&](auto &_holders_row){ - _holders_row.owner = to; - }); - } - } - //This is needed for sending notifications later if (collection_to_assets_transferred.find(asset_itr->collection_name) != collection_to_assets_transferred.end()) { @@ -1893,6 +2070,47 @@ void atomicassets::internal_transfer( } } + +/** +* Reverts if the asset has a live lease/title record (i.e. is rental-locked). +* The lock is keyed purely on the existence of a leases row, so it covers both +* pretitle sentinels and active leases with no post-expiry abscondment window. +*/ +void atomicassets::check_not_leased(uint64_t asset_id) { + leases_t leases = get_leases(); + check(leases.find(asset_id) == leases.end(), + ("Asset is leased and locked (ID: " + to_string(asset_id) + ")").c_str()); +} + + +/** +* Erases any offers created by `account` whose sender_asset_ids reference +* `asset_id`. Offers are AtomicAssets' approval mechanism, so this closes the +* stale-approval escape at lease-start and reclaim. +*/ +void atomicassets::clear_offers_for_asset(name account, uint64_t asset_id) { + auto offers = get_offers(); + auto offers_by_sender = offers.get_index(); + + auto itr = offers_by_sender.lower_bound(account.value); + auto end_itr = offers_by_sender.upper_bound(account.value); + while (itr != end_itr) { + bool references_asset = false; + for (uint64_t id : itr->sender_asset_ids) { + if (id == asset_id) { + references_asset = true; + break; + } + } + if (references_asset) { + itr = offers_by_sender.erase(itr); + } else { + ++itr; + } + } +} + + /** * Decreases the balance of a specified account by a specified quantity * If the specified account does not have at least as much tokens in the balance as should be removed diff --git a/tests/Admin Actions/init.test.js b/tests/Admin Actions/init.test.js index e56a958..d5b5a38 100644 --- a/tests/Admin Actions/init.test.js +++ b/tests/Admin Actions/init.test.js @@ -28,7 +28,8 @@ describe('test init contract', () => { "template_counter": 1, "offer_counter": 1, "collection_format": [], - "supported_tokens": [] + "supported_tokens": [], + "rental_market": "atomicmarket" }); }); @@ -50,7 +51,8 @@ describe('test init contract', () => { "template_counter": 1, "offer_counter": 1, "collection_format": [{"name": "name", "type": "string"}], - "supported_tokens": [] + "supported_tokens": [], + "rental_market": "atomicmarket" }); }); diff --git a/tests/Asset Actions/move.test.js b/tests/Asset Actions/move.test.js deleted file mode 100644 index 265fa60..0000000 --- a/tests/Asset Actions/move.test.js +++ /dev/null @@ -1,504 +0,0 @@ -const { Blockchain, nameToBigInt, mintTokens, bigIntToName } = require("@vaulta/vert"); -const { Name } = require('@wharfkit/antelope'); -const fs = require('fs'); - -describe('test move asset', () => { - let blockchain; - let eosioToken; - let atomicassets; - let user1; - let user2; - let user3; - - beforeAll(async () => { - blockchain = new Blockchain(); - atomicassets = blockchain.createContract( - 'atomicassets', - './build/atomicassets' - ); - eosioToken = blockchain.createAccount({ - name: Name.from('eosio.token'), - wasm: fs.readFileSync('./tests/fixtures/eosio.token/eosio.token.wasm'), - abi: fs.readFileSync('./tests/fixtures/eosio.token/eosio.token.abi', 'utf8'), - }); - user1 = blockchain.createAccount('user1'); - user2 = blockchain.createAccount('user2'); - user3 = blockchain.createAccount('user3'); - }); - - beforeEach(async () => { - blockchain.resetTables(); - await atomicassets.actions.init([]).send(`${atomicassets.name.toString()}@active`); - await mintTokens(eosioToken, 'WAX', 8, 1000000000, 10000, [user1, user2, user3]); - await mintTokens(eosioToken, 'EOS', 4, 1000000000, 10000, [user1, user2, user3]); - - await atomicassets.actions.createcol([ - user1.name.toString(), - "testcollect1", - true, - [user1.name.toString()], - [], - 0.05, - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.createschema([ - user1.name.toString(), - "testcollect1", - "testschema", - [ - {name: "name", type: "string"}, - {name: "level", type: "uint32"}, - {name: "img", type: "ipfs"} - ] - ]).send(`${user1.name.toString()}@active`); - }); - - test("throw if missing owner permission", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - ["1099511627776"], - '' - ]).send(`${user2.name.toString()}@active`)).rejects.toThrow('missing required authority user1'); - }); - - test("throw if from account does not exist", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - "nonexistent", - user2.name.toString(), - ["1099511627776"], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow('from account does not exist'); - }); - - test("throw if to account does not exist", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - "nonexistent", - ["1099511627776"], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow('to account does not exist'); - }); - - test("throw if from and to are the same", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user1.name.toString(), - ["1099511627776"], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow('from & to fields cannot be the same'); - }); - - test("throw if asset_ids is empty", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - [], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow('asset_ids needs to contain at least one id'); - }); - - test("throw if memo is too long", async () => { - const longMemo = 'a'.repeat(257); // 257 characters > 256 limit - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - ["1099511627776"], - longMemo - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow('A move memo can only be 256 characters max'); - }); - - test("throw if duplicate asset IDs provided", async () => { - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - ["1099511627776", "1099511627776"], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow("Can't move the same asset multiple times"); - }); - - test("throw if owner doesn't own the asset", async () => { - // Create template and mint asset to user2 - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, - user2.name.toString(), // mint to user2 - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // user1 tries to move asset they don't own - await expect(atomicassets.actions.move([ - user1.name.toString(), // user1 claims ownership - user2.name.toString(), // from user2 - user3.name.toString(), // to user3 - ["1099511627776"], - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow("Owner doesn't own at least one of the provided assets"); - }); - - test("throw if asset is not transferable", async () => { - // Create non-transferable template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - false, // not transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 (non-transferable) - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - ["1099511627776"], // asset_id of first minted asset - '' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow("At least one asset isn't transferable"); - }); - - test("throw if holder constraint violated", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // First move from owner (user1) to holder (user2) - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Initial move to holder' - ]).send(`${user1.name.toString()}@active`); - - // Try to move from wrong holder (user3 instead of user2) - await expect(atomicassets.actions.move([ - user1.name.toString(), // owner - user3.name.toString(), // from (wrong holder) - user1.name.toString(), // to (back to owner) - ["1099511627776"], - 'Wrong holder attempt' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow("At least one asset invalidates the 'from:holder' constraint"); - }); - - test("successfully move asset from owner to holder", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // Move from owner to holder - await expect(atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Move to holder' - ]).send(`${user1.name.toString()}@active`)).resolves.not.toThrow(); - - // Verify holder entry was created - const holdersTable = atomicassets.tables.holders(nameToBigInt(atomicassets.name)); - const holderEntry = holdersTable.getTableRow('1099511627776'); - expect(holderEntry).toBeDefined(); - expect(holderEntry.owner).toBe(user1.name.toString()); - expect(holderEntry.holder).toBe(user2.name.toString()); - - const expectLogmoveAction = blockchain.executionTraces[1]; - expect(expectLogmoveAction.contract.toString()).toBe(atomicassets.name.toString()); - expect(expectLogmoveAction.action.toString()).toBe('logmove'); - expect(expectLogmoveAction.data.collection_name.toString()).toBe('testcollect1'); - expect(expectLogmoveAction.data.owner.toString()).toBe(user1.name.toString()); - expect(expectLogmoveAction.data.from.toString()).toBe(user1.name.toString()); - expect(expectLogmoveAction.data.to.toString()).toBe(user2.name.toString()); - expect(expectLogmoveAction.data.asset_ids.length).toBe(1); - expect(expectLogmoveAction.data.asset_ids[0].toString()).toBe("1099511627776"); - expect(expectLogmoveAction.data.memo).toBe('Move to holder'); - }); - - test("successfully move asset between holders", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // First move from owner to holder - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Initial move to holder' - ]).send(`${user1.name.toString()}@active`); - - // Move between holders - await expect(atomicassets.actions.move([ - user1.name.toString(), // owner - user2.name.toString(), // from (current holder) - user3.name.toString(), // to (new holder) - ["1099511627776"], - 'Move between holders' - ]).send(`${user1.name.toString()}@active`)).resolves.not.toThrow(); - - // Verify holder entry was updated - const holdersTable = atomicassets.tables.holders(nameToBigInt(atomicassets.name)); - const holderEntry = holdersTable.getTableRow('1099511627776'); - expect(holderEntry).toBeDefined(); - expect(holderEntry.owner).toBe(user1.name.toString()); - expect(holderEntry.holder).toBe(user3.name.toString()); - }); - - test("successfully move asset from holder back to owner", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // First move from owner to holder - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Initial move to holder' - ]).send(`${user1.name.toString()}@active`); - - // Move back to owner - await expect(atomicassets.actions.move([ - user1.name.toString(), // owner - user2.name.toString(), // from (current holder) - user1.name.toString(), // to (back to owner) - ["1099511627776"], - 'Return to owner' - ]).send(`${user1.name.toString()}@active`)).resolves.not.toThrow(); - - // Verify holder entry was deleted - const holdersTable = atomicassets.tables.holders(nameToBigInt(atomicassets.name)); - const holderEntry = holdersTable.getTableRow('1099511627776'); - expect(holderEntry).toBeUndefined(); - }); - - test("successfully move multiple assets", async () => { - // Create template and mint multiple assets - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // Move multiple assets - await expect(atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776", "1099511627777"], // multiple assets - 'Move multiple assets' - ]).send(`${user1.name.toString()}@active`)).resolves.not.toThrow(); - - // Verify both holder entries were created - const holdersTable = atomicassets.tables.holders(nameToBigInt(atomicassets.name)); - const holderEntry1 = holdersTable.getTableRow('1099511627776'); - const holderEntry2 = holdersTable.getTableRow('1099511627777'); - - expect(holderEntry1).toBeDefined(); - expect(holderEntry1.owner).toBe(user1.name.toString()); - expect(holderEntry1.holder).toBe(user2.name.toString()); - - expect(holderEntry2).toBeDefined(); - expect(holderEntry2.owner).toBe(user1.name.toString()); - expect(holderEntry2.holder).toBe(user2.name.toString()); - }); - - test("throw if only owner can move from owner position", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - // user2 tries to move asset from user1 (owner) but user2 is not the owner - await expect(atomicassets.actions.move([ - user1.name.toString(), - user2.name.toString(), // should be user1 - user3.name.toString(), // to - ["1099511627776"], - 'Unauthorized move attempt' - ]).send(`${user1.name.toString()}@active`)).rejects.toThrow("Only the owner can move this asset"); - }); - - test("accept memo up to 256 characters", async () => { - // Create template and mint asset - await atomicassets.actions.createtempl([ - user1.name.toString(), - "testcollect1", - "testschema", - true, // transferable - true, // burnable - 0, // max_supply (unlimited) - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - 1, // template_id: 1 - user1.name.toString(), - [], // immutable_data - [], // mutable_data - [] // tokens_to_back - ]).send(`${user1.name.toString()}@active`); - - const validMemo = 'a'.repeat(256); // Exactly 256 characters - await expect(atomicassets.actions.move([ - user1.name.toString(), - user1.name.toString(), - user2.name.toString(), - ["1099511627776"], - validMemo - ]).send(`${user1.name.toString()}@active`)).resolves.not.toThrow(); - }); -}); \ No newline at end of file diff --git a/tests/Asset Actions/renting-invariants.test.js b/tests/Asset Actions/renting-invariants.test.js index 3296aa5..d41a627 100644 --- a/tests/Asset Actions/renting-invariants.test.js +++ b/tests/Asset Actions/renting-invariants.test.js @@ -1,200 +1,371 @@ -const { Blockchain, nameToBigInt, mintTokens } = require("@vaulta/vert"); -const { Name } = require('@wharfkit/antelope'); -const fs = require('fs'); - -// GAP-FILL (audit: A-BURN-RENTED, A-XFER-RENTED). The `move` action records a -// "holdership" (a holders row: asset_id + owner + holder) without locking the -// underlying asset. These tests LOCK the CURRENT on-chain behavior of what -// happens to a rented (held) asset when the OWNER burns or transfers it out -// from under the holder, so the pre-mainnet invariant decision is -// regression-guarded. They are characterization tests: they assert what the -// contract does today, not what it ideally should do. +const { Blockchain, nameToBigInt } = require("@vaulta/vert"); +const { TimePoint } = require("@wharfkit/antelope"); + +// Non-custodial "renter-as-owner" rental primitives. In this model the renter +// becomes the real AtomicAssets owner during a lease; the lister's reclaim right +// is parked in a `leases` row (the "title"); the asset is LOCKED (no +// transfer/burn/offer-out) while that row exists; and a permissionless `reclaim` +// force-returns it to the title_owner at expiry. // -// Current behavior (atomicassets.cpp): -// burnasset: holders row for the asset is ERASED, asset is burned. The -// holder silently loses the asset; no guard prevents this. -// internal_transfer: if `to` == holder, the holders row is ERASED (rental -// effectively settles to the holder). Otherwise the holders -// row's `owner` is REWRITTEN to the new owner and the -// holdership PERSISTS across the transfer. -describe("renting invariants characterization (burn / transfer of a held asset)", () => { +// These tests cover: the lock guards on every renter-reachable extraction path, +// the pretitle/leasestart/leaseextend lifecycle, the config-whitelisted market +// authority, the permissionless reclaim, and offer-clearing on lease-start. +describe("non-custodial rental primitives", () => { let blockchain; let atomicassets; - let eosioToken; - let owner; // asset owner / lessor - let holder; // current holder / lessee - let third; // unrelated third party + let market; // configured rental market (default config = "atomicmarket") + let lister; // title_owner / lessor + let renter; // becomes the AA owner during the lease + let third; // unrelated third party / random reclaim caller + + const ASSET1 = "1099511627776"; // 2^40, first minted asset id + const ONE_HOUR = 3600; + + function nowSec() { + return Math.floor(blockchain.timestamp.toMilliseconds() / 1000); + } + function leases() { + return atomicassets.tables.leases(nameToBigInt(atomicassets.name)).getTableRows(); + } + function assetsOf(account) { + return atomicassets.tables.assets(nameToBigInt(account.name)).getTableRows(); + } + function offers() { + return atomicassets.tables.offers(nameToBigInt(atomicassets.name)).getTableRows(); + } beforeAll(async () => { blockchain = new Blockchain(); - atomicassets = blockchain.createContract( - 'atomicassets', - './build/atomicassets' - ); - eosioToken = blockchain.createAccount({ - name: Name.from('eosio.token'), - wasm: fs.readFileSync('./tests/fixtures/eosio.token/eosio.token.wasm'), - abi: fs.readFileSync('./tests/fixtures/eosio.token/eosio.token.abi', 'utf8'), - }); - owner = blockchain.createAccount('user1'); - holder = blockchain.createAccount('user2'); - third = blockchain.createAccount('user3'); + atomicassets = blockchain.createContract('atomicassets', './build/atomicassets'); + // The default config.rental_market is "atomicmarket", so create that + // account as the authorized market. + market = blockchain.createAccount('atomicmarket'); + lister = blockchain.createAccount('lister'); + renter = blockchain.createAccount('renter'); + third = blockchain.createAccount('thirduser11'); }); beforeEach(async () => { blockchain.resetTables(); await atomicassets.actions.init([]).send(`${atomicassets.name.toString()}@active`); - await mintTokens(eosioToken, 'WAX', 8, 1000000000, 10000, [owner, holder, third]); await atomicassets.actions.createcol([ - owner.name.toString(), + lister.name.toString(), "testcollect1", true, - [owner.name.toString()], + [lister.name.toString()], [], 0.05, [] - ]).send(`${owner.name.toString()}@active`); + ]).send(`${lister.name.toString()}@active`); await atomicassets.actions.createschema([ - owner.name.toString(), + lister.name.toString(), "testcollect1", "testschema", [ - {name: "name", type: "string"}, - {name: "level", type: "uint32"}, - {name: "img", type: "ipfs"} + { name: "name", type: "string" }, + { name: "level", type: "uint32" }, + { name: "img", type: "ipfs" } ] - ]).send(`${owner.name.toString()}@active`); + ]).send(`${lister.name.toString()}@active`); - // Transferable + burnable template so move/transfer/burn are all allowed. + // template 1: transferable + burnable await atomicassets.actions.createtempl([ - owner.name.toString(), + lister.name.toString(), "testcollect1", "testschema", true, // transferable true, // burnable - 0, // max_supply + 0, [] - ]).send(`${owner.name.toString()}@active`); + ]).send(`${lister.name.toString()}@active`); + + // template 2: NON-transferable + await atomicassets.actions.createtempl([ + lister.name.toString(), + "testcollect1", + "testschema", + false, // transferable + true, // burnable + 0, + [] + ]).send(`${lister.name.toString()}@active`); }); - // Mints one asset to `owner` and moves it out to `holder`, creating the - // holders row. Returns the asset_id. - async function mintAndRent() { + // Mints one asset of the given template to the lister and returns its id. + async function mint(templateId = 1) { await atomicassets.actions.mintasset([ - owner.name.toString(), + lister.name.toString(), "testcollect1", "testschema", - 1, - owner.name.toString(), + templateId, + lister.name.toString(), [], [], [] - ]).send(`${owner.name.toString()}@active`); - - const assetId = "1099511627776"; - - await atomicassets.actions.move([ - owner.name.toString(), // owner - owner.name.toString(), // from (owner) - holder.name.toString(), // to (new holder) - [assetId], - 'Rent out asset' - ]).send(`${owner.name.toString()}@active`); - - // Holders row exists, owner still owns the asset row. - const holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: assetId, - owner: owner.name.toString(), - holder: holder.name.toString() - }); + ]).send(`${lister.name.toString()}@active`); + return ASSET1; + } - return assetId; + // Opens a lease directly (no prior pretitle) for the given duration. + async function leaseFor(seconds = ONE_HOUR) { + const rentalEnd = nowSec() + seconds; + await atomicassets.actions.leasestart([ + market.name.toString(), + lister.name.toString(), + renter.name.toString(), + ASSET1, + rentalEnd, + "lease start" + ]).send(`${market.name.toString()}@active`); + return rentalEnd; } - // A-BURN-RENTED: the OWNER can burn an asset that is currently held out by a - // lessee. There is NO guard. The asset is burned and the holders row is - // erased; the holder is left with nothing. - test("CURRENT BEHAVIOR: owner can burn a rented-out asset (holder loses it)", async () => { - const assetId = await mintAndRent(); + // ---------------------------------------------------------------- lifecycle - // Owner burns the held asset (no rejection). - await expect(atomicassets.actions.burnasset([ - owner.name.toString(), - assetId - ]).send(`${owner.name.toString()}@active`)).resolves.not.toThrow(); + test("pretitle locks the asset but leaves ownership with the lister", async () => { + await mint(); + await atomicassets.actions.pretitle([ + lister.name.toString(), market.name.toString(), ASSET1 + ]).send(`${lister.name.toString()}@active`); - // Asset is gone from the owner's scope. - const ownerAssets = atomicassets.tables.assets(nameToBigInt(owner.name)).getTableRows(); - expect(ownerAssets).toEqual([]); + // owner unchanged + expect(assetsOf(lister)).toHaveLength(1); + expect(assetsOf(renter)).toHaveLength(0); + // sentinel lease row present + expect(leases()).toEqual([{ + asset_id: ASSET1, + title_owner: lister.name.toString(), + renter: "", + rental_end: 0, + market: market.name.toString() + }]); + }); - // Holder never had an asset row in their scope (move only records - // holdership, it does not move the asset row). - const holderAssets = atomicassets.tables.assets(nameToBigInt(holder.name)).getTableRows(); - expect(holderAssets).toEqual([]); + test("leasestart makes the renter the real owner and records the title", async () => { + await mint(); + const rentalEnd = await leaseFor(); - // Holders row was erased by the burn. - const holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toEqual([]); + // ownership flipped lister -> renter + expect(assetsOf(lister)).toHaveLength(0); + expect(assetsOf(renter)).toHaveLength(1); + expect(assetsOf(renter)[0]).toMatchObject({ asset_id: ASSET1 }); + // active lease row + expect(leases()).toEqual([{ + asset_id: ASSET1, + title_owner: lister.name.toString(), + renter: renter.name.toString(), + rental_end: rentalEnd, + market: market.name.toString() + }]); }); - // A-XFER-RENTED (transfer to an unrelated third party, NOT the holder): - // the OWNER can transfer a held-out asset to someone else. The holders row - // is NOT erased; instead its `owner` field is rewritten to the new owner and - // the holdership PERSISTS. The asset row moves to the new owner's scope. - test("CURRENT BEHAVIOR: owner transfers a rented-out asset to a third party (holdership persists, owner rewritten)", async () => { - const assetId = await mintAndRent(); + test("leasestart can activate an existing pretitle sentinel", async () => { + await mint(); + await atomicassets.actions.pretitle([ + lister.name.toString(), market.name.toString(), ASSET1 + ]).send(`${lister.name.toString()}@active`); + const rentalEnd = await leaseFor(); - // Owner transfers the held asset to `third` (not the holder). - await expect(atomicassets.actions.transfer([ - owner.name.toString(), - third.name.toString(), - [assetId], - 'Sell rented asset out from under holder' - ]).send(`${owner.name.toString()}@active`)).resolves.not.toThrow(); - - // Asset row moved owner -> third. - const ownerAssets = atomicassets.tables.assets(nameToBigInt(owner.name)).getTableRows(); - expect(ownerAssets).toEqual([]); - const thirdAssets = atomicassets.tables.assets(nameToBigInt(third.name)).getTableRows(); - expect(thirdAssets).toHaveLength(1); - expect(thirdAssets[0]).toMatchObject({ asset_id: assetId }); - - // Holders row PERSISTS; owner rewritten to `third`, holder unchanged. - const holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: assetId, - owner: third.name.toString(), - holder: holder.name.toString() + expect(assetsOf(renter)).toHaveLength(1); + expect(leases()[0]).toMatchObject({ + renter: renter.name.toString(), + rental_end: rentalEnd }); }); - // A-XFER-RENTED (transfer TO the current holder): the rental "settles" — the - // holders row is erased and the asset row moves to the holder, who now owns - // it outright. - test("CURRENT BEHAVIOR: owner transfers a rented-out asset to the holder (holdership settles)", async () => { - const assetId = await mintAndRent(); + test("throw when leasing an already-leased asset", async () => { + await mint(); + await leaseFor(); + const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + market.name.toString(), lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "second lease" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("already leased"); + }); + + test("throw when leasing a non-transferable asset", async () => { + await mint(2); // non-transferable template + const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + market.name.toString(), lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "lease" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("not transferable"); + }); + + test("leaseextend bumps the end without changing ownership", async () => { + await mint(); + await leaseFor(); + const newEnd = nowSec() + ONE_HOUR * 5; + await atomicassets.actions.leaseextend([ + market.name.toString(), ASSET1, newEnd + ]).send(`${market.name.toString()}@active`); + + expect(assetsOf(renter)).toHaveLength(1); // still the renter's + expect(leases()[0].rental_end).toBe(newEnd); + }); + + // -------------------------------------------------------------- lock guards + + test("a leased asset cannot be transferred by the renter (its owner)", async () => { + await mint(); + await leaseFor(); + await expect(atomicassets.actions.transfer([ + renter.name.toString(), third.name.toString(), [ASSET1], "escape" + ]).send(`${renter.name.toString()}@active`)).rejects.toThrow("leased and locked"); + }); + + test("a leased asset cannot be burned", async () => { + await mint(); + await leaseFor(); + await expect(atomicassets.actions.burnasset([ + renter.name.toString(), ASSET1 + ]).send(`${renter.name.toString()}@active`)).rejects.toThrow("leased and locked"); + }); + + test("a leased asset cannot be offered out by the renter", async () => { + await mint(); + await leaseFor(); + await expect(atomicassets.actions.createoffer([ + renter.name.toString(), third.name.toString(), [ASSET1], [], "" + ]).send(`${renter.name.toString()}@active`)).rejects.toThrow("leased and locked"); + }); + test("a pretitle-locked asset cannot be transferred by the lister", async () => { + await mint(); + await atomicassets.actions.pretitle([ + lister.name.toString(), market.name.toString(), ASSET1 + ]).send(`${lister.name.toString()}@active`); await expect(atomicassets.actions.transfer([ - owner.name.toString(), - holder.name.toString(), - [assetId], - 'Settle rental to holder' - ]).send(`${owner.name.toString()}@active`)).resolves.not.toThrow(); - - // Asset row moved owner -> holder. - const ownerAssets = atomicassets.tables.assets(nameToBigInt(owner.name)).getTableRows(); - expect(ownerAssets).toEqual([]); - const holderAssets = atomicassets.tables.assets(nameToBigInt(holder.name)).getTableRows(); - expect(holderAssets).toHaveLength(1); - expect(holderAssets[0]).toMatchObject({ asset_id: assetId }); - - // Holders row erased (rental settled to holder). - const holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toEqual([]); + lister.name.toString(), third.name.toString(), [ASSET1], "" + ]).send(`${lister.name.toString()}@active`)).rejects.toThrow("leased and locked"); + }); + + test("DELIBERATE NON-GUARD: collection can still setassetdata on a leased asset", async () => { + await mint(); + await leaseFor(); + // setassetdata is collection-auth gated, never renter-reachable, and only + // mutates metadata — it must keep working during a lease. + await expect(atomicassets.actions.setassetdata([ + lister.name.toString(), // authorized_editor (collection auth) + renter.name.toString(), // asset_owner (the renter, now the owner) + ASSET1, + [{ "first": "name", "second": ["string", "leased-but-editable"] }] + ]).send(`${lister.name.toString()}@active`)).resolves.not.toThrow(); + }); + + test("NO REGRESSION: an unleased asset transfers normally", async () => { + await mint(); + await expect(atomicassets.actions.transfer([ + lister.name.toString(), renter.name.toString(), [ASSET1], "" + ]).send(`${lister.name.toString()}@active`)).resolves.not.toThrow(); + expect(assetsOf(renter)).toHaveLength(1); + }); + + // ----------------------------------------------------------------- authority + + test("throw when an unconfigured account tries to open a lease", async () => { + await mint(); + const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + third.name.toString(), lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "lease" + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("not the configured rental market"); + }); + + test("setrentmkt requires contract authority and reconfigures the market", async () => { + await mint(); + await expect(atomicassets.actions.setrentmkt([ + third.name.toString() + ]).send(`${lister.name.toString()}@active`)).rejects.toThrow("missing required authority"); + + // Re-point the market to `third`, who can now open leases. + await atomicassets.actions.setrentmkt([ + third.name.toString() + ]).send(`${atomicassets.name.toString()}@active`); + + const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + third.name.toString(), lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "lease" + ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); + expect(assetsOf(renter)).toHaveLength(1); + }); + + // ------------------------------------------------------------------- reclaim + + test("throw when reclaiming before expiry", async () => { + await mint(); + await leaseFor(ONE_HOUR); + await expect(atomicassets.actions.reclaim([ + ASSET1 + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("has not expired"); + }); + + test("anyone can reclaim after expiry, returning the asset to the lister", async () => { + await mint(); + await leaseFor(ONE_HOUR); + + blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); + + // a random, unrelated account triggers the reclaim + await expect(atomicassets.actions.reclaim([ + ASSET1 + ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); + + expect(assetsOf(renter)).toHaveLength(0); + expect(assetsOf(lister)).toHaveLength(1); + expect(assetsOf(lister)[0]).toMatchObject({ asset_id: ASSET1 }); + expect(leases()).toEqual([]); // lock cleared + }); + + test("delpretitle clears a sentinel; reclaim refuses a sentinel", async () => { + await mint(); + await atomicassets.actions.pretitle([ + lister.name.toString(), market.name.toString(), ASSET1 + ]).send(`${lister.name.toString()}@active`); + + // reclaim is for expired leases, not sentinels + await expect(atomicassets.actions.reclaim([ + ASSET1 + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("not leased; use delpretitle"); + + await atomicassets.actions.delpretitle([ + ASSET1 + ]).send(`${lister.name.toString()}@active`); + expect(leases()).toEqual([]); + // unlocked again + await expect(atomicassets.actions.transfer([ + lister.name.toString(), renter.name.toString(), [ASSET1], "" + ]).send(`${lister.name.toString()}@active`)).resolves.not.toThrow(); + }); + + // ------------------------------------------------------------ offer clearing + + test("leasestart clears a stale offer the lister created for the asset", async () => { + await mint(); + // lister lists the asset in an out-offer BEFORE leasing + await atomicassets.actions.createoffer([ + lister.name.toString(), third.name.toString(), [ASSET1], [], "" + ]).send(`${lister.name.toString()}@active`); + expect(offers()).toHaveLength(1); + + await leaseFor(); + + // the stale offer is gone, so it can never settle around the lock + expect(offers()).toEqual([]); + }); + + test("reclaim clears a stale offer the renter created for the asset", async () => { + await mint(); + await leaseFor(ONE_HOUR); + + // Force a stale renter offer into the table directly is not possible + // (createoffer is guarded), so this asserts the table stays clean across + // a reclaim — the renter could never have created one. + blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); + await atomicassets.actions.reclaim([ASSET1]).send(`${third.name.toString()}@active`); + expect(offers()).toEqual([]); }); }); diff --git a/tests/Deposit-Withdraw-Back-Burn Actions/burnasset.test.js b/tests/Deposit-Withdraw-Back-Burn Actions/burnasset.test.js index 07ce195..7162be9 100644 --- a/tests/Deposit-Withdraw-Back-Burn Actions/burnasset.test.js +++ b/tests/Deposit-Withdraw-Back-Burn Actions/burnasset.test.js @@ -187,48 +187,4 @@ describe("test burnasset contract", () => { "1099511627776" ]).send(`${user2.name.toString()}@active`)).rejects.toThrow("missing required authority"); }); - - test("burn asset with holder record deletes the holder entry", async () => { - expect.assertions(3); - - // Mint asset for user1 - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - // Move asset from owner (user1) to holder (user2) - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Move to holder for burning test' - ]).send(`${user1.name.toString()}@owner`); - - // Verify holder record exists - let holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: "1099511627776", - owner: user1.name.toString(), - holder: user2.name.toString() - }); - - // Burn the asset (owner can burn even when held by someone else) - await atomicassets.actions.burnasset([ - user1.name.toString(), - "1099511627776" - ]).send(`${user1.name.toString()}@active`); - - // Verify holder record was deleted along with the asset - holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(0); - }); }); \ No newline at end of file diff --git a/tests/Transfer-Offer Actions/transfer.test.js b/tests/Transfer-Offer Actions/transfer.test.js index c7d830f..c4a3109 100644 --- a/tests/Transfer-Offer Actions/transfer.test.js +++ b/tests/Transfer-Offer Actions/transfer.test.js @@ -528,211 +528,4 @@ describe('test transfer contract', () => { "" ]).send(`${user2.name.toString()}@active`)).rejects.toThrow("missing required authority"); }); - - test("transfer asset with holder record - transfer to holder deletes holder entry", async () => { - // Mint asset for user1 - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - // Move asset from owner (user1) to holder (user2) using move action - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Create holder relationship for transfer test' - ]).send(`${user1.name.toString()}@owner`); - - // Verify holder record exists - let holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: "1099511627776", - owner: user1.name.toString(), - holder: user2.name.toString() - }); - - // Transfer asset from owner (user1) to the current holder (user2) - // This should delete the holder record since we're transferring to the holder - await atomicassets.actions.transfer([ - user1.name.toString(), // from (owner) - user2.name.toString(), // to (current holder) - ["1099511627776"], - "Transfer to current holder" - ]).send(`${user1.name.toString()}@active`); - - // Verify holder record was deleted - holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(0); - - // Verify asset is now owned by user2 - const user2_assets = atomicassets.tables.assets(nameToBigInt(user2.name)).getTableRows(); - expect(user2_assets).toHaveLength(1); - expect(user2_assets[0].asset_id).toBe("1099511627776"); - }); - - test("transfer asset with holder record - transfer to new owner updates holder ownership", async () => { - const user3 = blockchain.createAccount('user3'); - - // Mint asset for user1 - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - // Move asset from owner (user1) to holder (user2) - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], - 'Create holder relationship for transfer test' - ]).send(`${user1.name.toString()}@owner`); - - // Verify initial holder record - let holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: "1099511627776", - owner: user1.name.toString(), - holder: user2.name.toString() - }); - - // Transfer asset from owner (user1) to new owner (user3) - // This should update the holder record to show user3 as the new owner - await atomicassets.actions.transfer([ - user1.name.toString(), // from (current owner) - user3.name.toString(), // to (new owner) - ["1099511627776"], - "Transfer to new owner while held by someone else" - ]).send(`${user1.name.toString()}@active`); - - // Verify holder record was updated with new ownership - holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: "1099511627776", - owner: user3.name.toString(), // updated to new owner - holder: user2.name.toString() // holder remains the same - }); - - // Verify asset is now owned by user3 - const user3_assets = atomicassets.tables.assets(nameToBigInt(user3.name)).getTableRows(); - expect(user3_assets).toHaveLength(1); - expect(user3_assets[0].asset_id).toBe("1099511627776"); - }); - - test("transfer asset without holder record - no holder table interactions", async () => { - // Mint asset for user1 (no holder relationship created) - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - // Verify no holder records exist initially - let holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(0); - - // Transfer asset normally (owner to new owner, no holder involved) - await atomicassets.actions.transfer([ - user1.name.toString(), - user2.name.toString(), - ["1099511627776"], - "Normal transfer without holder" - ]).send(`${user1.name.toString()}@active`); - - // Verify still no holder records (normal transfer case) - holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(0); - - // Verify asset was transferred successfully - const user2_assets = atomicassets.tables.assets(nameToBigInt(user2.name)).getTableRows(); - expect(user2_assets).toHaveLength(1); - expect(user2_assets[0].asset_id).toBe("1099511627776"); - }); - - test("transfer multiple assets with mixed holder scenarios", async () => { - const user3 = blockchain.createAccount('user3'); - - // Mint two assets for user1 - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - await atomicassets.actions.mintasset([ - user1.name.toString(), - "testcollect1", - "testschema", - -1, - user1.name.toString(), - [], - [], - [] - ]).send(`${user1.name.toString()}@active`); - - // Create holder relationship for first asset only - await atomicassets.actions.move([ - user1.name.toString(), // owner - user1.name.toString(), // from (owner) - user2.name.toString(), // to (new holder) - ["1099511627776"], // only first asset - 'Create holder for first asset only' - ]).send(`${user1.name.toString()}@owner`); - - // Verify only one holder record exists - let holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0].asset_id).toBe("1099511627776"); - - // Transfer both assets to user3 - // First asset has holder (should update ownership) - // Second asset has no holder (normal transfer) - await atomicassets.actions.transfer([ - user1.name.toString(), - user3.name.toString(), - ["1099511627776", "1099511627777"], - "Transfer assets with mixed holder scenarios" - ]).send(`${user1.name.toString()}@active`); - - // Verify holder record was updated for first asset - holders = atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(); - expect(holders).toHaveLength(1); - expect(holders[0]).toMatchObject({ - asset_id: "1099511627776", - owner: user3.name.toString(), // ownership updated - holder: user2.name.toString() // holder unchanged - }); - - // Verify both assets are now owned by user3 - const user3_assets = atomicassets.tables.assets(nameToBigInt(user3.name)).getTableRows(); - expect(user3_assets).toHaveLength(2); - expect(user3_assets.map(a => a.asset_id).sort()).toEqual(["1099511627776", "1099511627777"]); - }); }); \ No newline at end of file From 63125d20b32bb37fc07a34047fdedd016c4f2c71 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 21:42:53 -0400 Subject: [PATCH 02/11] refactor(rentals): address review (drop pretitle, dedicated rentalcfg singleton) - Drop pretitle/delpretitle and the sentinel lease state. leasestart is always the direct trusted-market path (lister consent is captured by AtomicMarket's announcerent). A leases row now always means an active lease. - Store the configured rental market in a new rentalcfg singleton (default atomicmarket, read via get_or_default) instead of appending to config_s, so deploying onto the existing live contract needs no config migration. - Remove offer-clearing on lease-start/reclaim. A pre-existing offer now survives a rental (it cannot settle while the asset is locked) and becomes acceptable again after reclaim. This also removes a renter-controlled unbounded loop on the permissionless reclaim path. - Add a check_rental_market helper; remove the dead renter=="" re-checks. - Update the VeRT suite (lease lifecycle, authority, reclaim, offer survival). --- include/atomicassets-interface.hpp | 1 - include/atomicassets.hpp | 47 ++--- src/atomicassets.cpp | 199 +++--------------- tests/Admin Actions/init.test.js | 6 +- .../Asset Actions/renting-invariants.test.js | 112 +++------- 5 files changed, 91 insertions(+), 274 deletions(-) diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index 6982cc3..64f867c 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -201,7 +201,6 @@ namespace atomicassets { uint64_t offer_counter = 1; vector collection_format = {}; vector supported_tokens = {}; - name rental_market = name("atomicmarket"); }; typedef singleton config_t; diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index d2fcb65..fa49705 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -38,12 +38,6 @@ CONTRACT atomicassets : public contract { name rental_market ); - ACTION pretitle( - name title_owner, - name market, - uint64_t asset_id - ); - ACTION leasestart( name market, name title_owner, @@ -59,10 +53,6 @@ CONTRACT atomicassets : public contract { uint32_t rental_end ); - ACTION delpretitle( - uint64_t asset_id - ); - ACTION reclaim( uint64_t asset_id ); @@ -470,19 +460,15 @@ CONTRACT atomicassets : public contract { typedef multi_index assets_t; - // Non-custodial rental "title" / lock record. The existence of a row for an - // asset_id means the asset is LOCKED (no transfer/burn/offer-out/sale). Two - // states: - // pretitle sentinel: renter == name(""), rental_end == 0 — the lister has - // pre-locked the asset and consented to `market`; owner is still the - // lister. - // active lease: renter != name(""), rental_end > 0 — the renter is the - // real AtomicAssets owner; title_owner holds the reclaim right. + // Non-custodial rental "title" / lock record. A row exists for an asset_id iff + // it is actively leased: the renter is the real AtomicAssets owner, the asset + // is LOCKED (no transfer/burn/offer-out/sale), and title_owner holds the + // reclaim right until rental_end. TABLE leases_s { uint64_t asset_id; name title_owner; // lister; reclaim returns the asset here name renter; // current AA owner during the lease - uint32_t rental_end; // sec_since_epoch; 0 for a pretitle sentinel + uint32_t rental_end; // sec_since_epoch the lease expires name market; // rental market that opened/manages the lease uint64_t primary_key() const { return asset_id; }; @@ -531,16 +517,21 @@ CONTRACT atomicassets : public contract { uint64_t offer_counter = 1; vector collection_format = {}; vector supported_tokens = {}; - // The single account authorized to open/manage non-custodial rental - // leases (pretitle/leasestart/leaseextend). Defaults to the AtomicMarket - // contract; set to name("") via setrentmkt to disable leasing. - // NOTE: appending this field requires re-initialising the config - // singleton on an existing deployment (the stored blob predates it). - name rental_market = name("atomicmarket"); }; typedef singleton config_t; + // The single account authorized to open/manage non-custodial rental leases + // (leasestart/leaseextend). Kept in its OWN singleton (not appended to config) + // so deploying onto an existing contract needs no config migration: an absent + // row reads as the default (the AtomicMarket contract). setrentmkt overwrites + // it; name("") disables leasing. + TABLE rentalcfg_s { + name rental_market = name("atomicmarket"); + }; + typedef singleton rentalcfg_t; + + TABLE tokenconfigs_s { name standard = name("atomicassets"); std::string version = string("2.0.0"); @@ -559,6 +550,7 @@ CONTRACT atomicassets : public contract { offers_t get_offers() {return offers_t(get_self(), get_self().value);} balances_t get_balances() {return balances_t(get_self(), get_self().value);} config_t get_config() {return config_t(get_self(), get_self().value);} + rentalcfg_t get_rentalcfg() {return rentalcfg_t(get_self(), get_self().value);} tokenconfigs_t get_tokenconfigs() {return tokenconfigs_t(get_self(), get_self().value);} schemas_t get_schemas(name collection_name) {return schemas_t(get_self(), collection_name.value);} @@ -614,9 +606,8 @@ CONTRACT atomicassets : public contract { // Reverts if the asset has a live lease/title record (i.e. is rental-locked). void check_not_leased(uint64_t asset_id); - // Erases any offers created by `account` whose sender_asset_ids reference - // `asset_id` (offers are AtomicAssets' approval mechanism). - void clear_offers_for_asset(name account, uint64_t asset_id); + // Asserts `market` is the configured rental market and requires its auth. + void check_rental_market(name market); void notify_collection_accounts( name collection_name diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index 17f2033..b59ba97 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -87,7 +87,8 @@ ACTION atomicassets::transfer( /** * Sets the single account authorized to open/manage non-custodial rental leases -* (pretitle / leasestart / leaseextend). name("") disables leasing entirely. +* (leasestart / leaseextend). name("") disables leasing entirely. Stored in its +* own `rentalcfg` singleton so it needs no migration of the existing config row. * @required_auth The contract itself */ ACTION atomicassets::setrentmkt(name rental_market) { @@ -96,69 +97,16 @@ ACTION atomicassets::setrentmkt(name rental_market) { check(rental_market == name("") || is_account(rental_market), "rental_market account does not exist"); - auto config = get_config(); - config_s current_config = config.get(); - current_config.rental_market = rental_market; - config.set(current_config, get_self()); -} - - -/** -* Pre-locks an asset for non-custodial rental and records the lister's consent -* to `market` BEFORE any renter exists. Owner stays the lister; the asset is -* immediately locked (a lease row exists). This is the lister-signed half of the -* hybrid lease-start handshake — the market can later activate a lease only on -* assets that carry such a row, capping the market's blast radius. -* @required_auth title_owner -*/ -ACTION atomicassets::pretitle( - name title_owner, - name market, - uint64_t asset_id -) { - require_auth(title_owner); - - auto config = get_config(); - config_s current_config = config.get(); - check(current_config.rental_market != name("") && market == current_config.rental_market, - "market is not the configured rental market"); - - assets_t owner_assets = get_assets(title_owner); - auto asset_itr = owner_assets.require_find(asset_id, - "title_owner does not own this asset"); - - // A non-transferable asset can never be leased out. - if (asset_itr->template_id >= 0) { - templates_t collection_templates = get_templates(asset_itr->collection_name); - auto template_itr = collection_templates.find(asset_itr->template_id); - check(template_itr->transferable, "The asset is not transferable"); - } - - leases_t leases = get_leases(); - check(leases.find(asset_id) == leases.end(), "Asset is already leased or pre-titled"); - - leases.emplace(title_owner, [&](auto &_lease) { - _lease.asset_id = asset_id; - _lease.title_owner = title_owner; - _lease.renter = name(""); - _lease.rental_end = 0; - _lease.market = market; - }); - - action( - permission_level{get_self(), name("active")}, - get_self(), - name("loglock"), - make_tuple(asset_itr->collection_name, asset_id, title_owner, name(""), (uint32_t) 0, market) - ).send(); + get_rentalcfg().set(rentalcfg_s{rental_market}, get_self()); } /** * Opens a non-custodial rental lease: makes `renter` the real AtomicAssets owner * of the asset and parks the lister's reclaim right in the lease record, with NO -* unlocked window (the lease row is written before the ownership flip). May -* activate an existing pretitle sentinel or open a lease directly. +* unlocked window (the lease row is written before the ownership flip). The +* configured rental market is trusted to have verified the lister's consent (on +* AtomicMarket the lister's announcerent carries that authorization). * @required_auth market (must be the configured rental_market) */ ACTION atomicassets::leasestart( @@ -169,12 +117,7 @@ ACTION atomicassets::leasestart( uint32_t rental_end, string memo ) { - require_auth(market); - - auto config = get_config(); - config_s current_config = config.get(); - check(current_config.rental_market != name("") && market == current_config.rental_market, - "market is not the configured rental market"); + check_rental_market(market); check(is_account(renter), "renter account does not exist"); check(renter != title_owner, "renter and title_owner cannot be the same"); @@ -182,12 +125,8 @@ ACTION atomicassets::leasestart( uint32_t now = eosio::current_time_point().sec_since_epoch(); check(rental_end > now, "rental_end must be in the future"); - // Reject an already-active lease up front (the title_owner no longer owns the - // asset in that state, so the ownership check below would mask the cause). leases_t leases = get_leases(); - auto lease_itr = leases.find(asset_id); - check(lease_itr == leases.end() || lease_itr->renter == name(""), - "Asset is already leased"); + check(leases.find(asset_id) == leases.end(), "Asset is already leased"); assets_t owner_assets = get_assets(title_owner); auto asset_itr = owner_assets.require_find(asset_id, @@ -202,33 +141,21 @@ ACTION atomicassets::leasestart( check(template_itr->transferable, "The asset is not transferable"); } - if (lease_itr == leases.end()) { - // Direct lease-start (no prior pretitle). Write the lock row FIRST. - leases.emplace(market, [&](auto &_lease) { - _lease.asset_id = asset_id; - _lease.title_owner = title_owner; - _lease.renter = renter; - _lease.rental_end = rental_end; - _lease.market = market; - }); - } else { - // Activating a pretitle sentinel. - check(lease_itr->title_owner == title_owner, "lease title_owner mismatch"); - check(lease_itr->renter == name(""), "Asset is already leased"); - leases.modify(lease_itr, market, [&](auto &_lease) { - _lease.renter = renter; - _lease.rental_end = rental_end; - _lease.market = market; - }); - } - - // Close the stale-offer escape: drop any offers the lister created that - // reference this asset before it changes hands. - clear_offers_for_asset(title_owner, asset_id); + // Write the lock row FIRST so there is no instant where the asset is + // renter-owned but unlocked. + leases.emplace(market, [&](auto &_lease) { + _lease.asset_id = asset_id; + _lease.title_owner = title_owner; + _lease.renter = renter; + _lease.rental_end = rental_end; + _lease.market = market; + }); // Flip ownership lister -> renter under the contract's own authority. The // lock is already in force, so this is the privileged (enforce_lock=false) - // path. The contract pays any transient scope RAM. + // path. The contract pays any transient scope RAM. A pre-existing offer that + // references the asset is intentionally left in place: it cannot settle while + // the asset is locked, and becomes valid again once the asset is reclaimed. internal_transfer(title_owner, renter, vector{asset_id}, memo, get_self(), false); action( @@ -249,16 +176,10 @@ ACTION atomicassets::leaseextend( uint64_t asset_id, uint32_t rental_end ) { - require_auth(market); - - auto config = get_config(); - config_s current_config = config.get(); - check(current_config.rental_market != name("") && market == current_config.rental_market, - "market is not the configured rental market"); + check_rental_market(market); leases_t leases = get_leases(); auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); - check(lease_itr->renter != name(""), "Asset is pre-titled but not leased"); check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); name title_owner = lease_itr->title_owner; @@ -281,37 +202,6 @@ ACTION atomicassets::leaseextend( } -/** -* Clears a pretitle sentinel (a pre-locked asset that was never leased), -* unlocking it. Only valid before a lease is opened. -* @required_auth title_owner -*/ -ACTION atomicassets::delpretitle( - uint64_t asset_id -) { - leases_t leases = get_leases(); - auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); - check(lease_itr->renter == name(""), - "Asset is actively leased; use reclaim after expiry"); - - require_auth(lease_itr->title_owner); - - name title_owner = lease_itr->title_owner; - assets_t owner_assets = get_assets(title_owner); - auto asset_itr = owner_assets.require_find(asset_id, "title_owner does not own this asset"); - name collection_name = asset_itr->collection_name; - - leases.erase(lease_itr); - - action( - permission_level{get_self(), name("active")}, - get_self(), - name("logreclaim"), - make_tuple(collection_name, asset_id, title_owner, name("")) - ).send(); -} - - /** * Permissionless reclaim of an expired lease: returns ownership from the renter * to the title_owner and clears the lock. Callable by anyone once the lease has @@ -325,8 +215,6 @@ ACTION atomicassets::reclaim( ) { leases_t leases = get_leases(); auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); - check(lease_itr->renter != name(""), - "Asset is pre-titled but not leased; use delpretitle"); uint32_t now = eosio::current_time_point().sec_since_epoch(); check(now >= lease_itr->rental_end, "Lease has not expired yet"); @@ -338,12 +226,11 @@ ACTION atomicassets::reclaim( auto asset_itr = renter_assets.require_find(asset_id, "renter no longer owns the asset"); name collection_name = asset_itr->collection_name; - // Close the stale-offer escape on the renter side before the asset moves. - clear_offers_for_asset(renter, asset_id); - - // Erase the lock, then move the asset back under the contract's own - // authority (enforce_lock=false). The contract pays any transient scope RAM - // so reclaim never needs the title_owner's signature. + // Erase the lock, then move the asset back under the contract's own authority + // (enforce_lock=false). The contract pays any transient scope RAM so reclaim + // never needs the title_owner's or renter's signature. A pre-existing offer + // referencing the asset is left in place (it could not settle while locked, + // and becomes valid again now that the asset is back with its owner). leases.erase(lease_itr); internal_transfer(renter, title_owner, vector{asset_id}, "lease reclaim", get_self(), false); @@ -2073,8 +1960,8 @@ void atomicassets::internal_transfer( /** * Reverts if the asset has a live lease/title record (i.e. is rental-locked). -* The lock is keyed purely on the existence of a leases row, so it covers both -* pretitle sentinels and active leases with no post-expiry abscondment window. +* The lock is keyed purely on the existence of a leases row, so there is no +* post-expiry abscondment window. */ void atomicassets::check_not_leased(uint64_t asset_id) { leases_t leases = get_leases(); @@ -2084,30 +1971,14 @@ void atomicassets::check_not_leased(uint64_t asset_id) { /** -* Erases any offers created by `account` whose sender_asset_ids reference -* `asset_id`. Offers are AtomicAssets' approval mechanism, so this closes the -* stale-approval escape at lease-start and reclaim. +* Asserts that `market` is the configured rental market and is authorized. The +* configured account is the single trusted opener/manager of leases. */ -void atomicassets::clear_offers_for_asset(name account, uint64_t asset_id) { - auto offers = get_offers(); - auto offers_by_sender = offers.get_index(); - - auto itr = offers_by_sender.lower_bound(account.value); - auto end_itr = offers_by_sender.upper_bound(account.value); - while (itr != end_itr) { - bool references_asset = false; - for (uint64_t id : itr->sender_asset_ids) { - if (id == asset_id) { - references_asset = true; - break; - } - } - if (references_asset) { - itr = offers_by_sender.erase(itr); - } else { - ++itr; - } - } +void atomicassets::check_rental_market(name market) { + name configured = get_rentalcfg().get_or_default(rentalcfg_s{}).rental_market; + check(configured != name("") && market == configured, + "market is not the configured rental market"); + require_auth(market); } diff --git a/tests/Admin Actions/init.test.js b/tests/Admin Actions/init.test.js index d5b5a38..e56a958 100644 --- a/tests/Admin Actions/init.test.js +++ b/tests/Admin Actions/init.test.js @@ -28,8 +28,7 @@ describe('test init contract', () => { "template_counter": 1, "offer_counter": 1, "collection_format": [], - "supported_tokens": [], - "rental_market": "atomicmarket" + "supported_tokens": [] }); }); @@ -51,8 +50,7 @@ describe('test init contract', () => { "template_counter": 1, "offer_counter": 1, "collection_format": [{"name": "name", "type": "string"}], - "supported_tokens": [], - "rental_market": "atomicmarket" + "supported_tokens": [] }); }); diff --git a/tests/Asset Actions/renting-invariants.test.js b/tests/Asset Actions/renting-invariants.test.js index d41a627..0b9fb0b 100644 --- a/tests/Asset Actions/renting-invariants.test.js +++ b/tests/Asset Actions/renting-invariants.test.js @@ -8,12 +8,13 @@ const { TimePoint } = require("@wharfkit/antelope"); // force-returns it to the title_owner at expiry. // // These tests cover: the lock guards on every renter-reachable extraction path, -// the pretitle/leasestart/leaseextend lifecycle, the config-whitelisted market -// authority, the permissionless reclaim, and offer-clearing on lease-start. +// the leasestart/leaseextend lifecycle, the configured-market authority (stored +// in the rentalcfg singleton), the permissionless reclaim, and the fact that a +// pre-existing offer survives a rental rather than being cleared. describe("non-custodial rental primitives", () => { let blockchain; let atomicassets; - let market; // configured rental market (default config = "atomicmarket") + let market; // configured rental market (rentalcfg default = "atomicmarket") let lister; // title_owner / lessor let renter; // becomes the AA owner during the lease let third; // unrelated third party / random reclaim caller @@ -37,8 +38,8 @@ describe("non-custodial rental primitives", () => { beforeAll(async () => { blockchain = new Blockchain(); atomicassets = blockchain.createContract('atomicassets', './build/atomicassets'); - // The default config.rental_market is "atomicmarket", so create that - // account as the authorized market. + // The rentalcfg singleton defaults to "atomicmarket", so create that + // account as the authorized market (no setrentmkt needed). market = blockchain.createAccount('atomicmarket'); lister = blockchain.createAccount('lister'); renter = blockchain.createAccount('renter'); @@ -108,7 +109,7 @@ describe("non-custodial rental primitives", () => { return ASSET1; } - // Opens a lease directly (no prior pretitle) for the given duration. + // Opens a lease (market-signed) for the given duration. async function leaseFor(seconds = ONE_HOUR) { const rentalEnd = nowSec() + seconds; await atomicassets.actions.leasestart([ @@ -124,25 +125,6 @@ describe("non-custodial rental primitives", () => { // ---------------------------------------------------------------- lifecycle - test("pretitle locks the asset but leaves ownership with the lister", async () => { - await mint(); - await atomicassets.actions.pretitle([ - lister.name.toString(), market.name.toString(), ASSET1 - ]).send(`${lister.name.toString()}@active`); - - // owner unchanged - expect(assetsOf(lister)).toHaveLength(1); - expect(assetsOf(renter)).toHaveLength(0); - // sentinel lease row present - expect(leases()).toEqual([{ - asset_id: ASSET1, - title_owner: lister.name.toString(), - renter: "", - rental_end: 0, - market: market.name.toString() - }]); - }); - test("leasestart makes the renter the real owner and records the title", async () => { await mint(); const rentalEnd = await leaseFor(); @@ -161,18 +143,10 @@ describe("non-custodial rental primitives", () => { }]); }); - test("leasestart can activate an existing pretitle sentinel", async () => { + test("leasing works out of the box on the default rentalcfg (no setrentmkt needed)", async () => { await mint(); - await atomicassets.actions.pretitle([ - lister.name.toString(), market.name.toString(), ASSET1 - ]).send(`${lister.name.toString()}@active`); - const rentalEnd = await leaseFor(); - + await expect(leaseFor()).resolves.toBeDefined(); expect(assetsOf(renter)).toHaveLength(1); - expect(leases()[0]).toMatchObject({ - renter: renter.name.toString(), - rental_end: rentalEnd - }); }); test("throw when leasing an already-leased asset", async () => { @@ -232,16 +206,6 @@ describe("non-custodial rental primitives", () => { ]).send(`${renter.name.toString()}@active`)).rejects.toThrow("leased and locked"); }); - test("a pretitle-locked asset cannot be transferred by the lister", async () => { - await mint(); - await atomicassets.actions.pretitle([ - lister.name.toString(), market.name.toString(), ASSET1 - ]).send(`${lister.name.toString()}@active`); - await expect(atomicassets.actions.transfer([ - lister.name.toString(), third.name.toString(), [ASSET1], "" - ]).send(`${lister.name.toString()}@active`)).rejects.toThrow("leased and locked"); - }); - test("DELIBERATE NON-GUARD: collection can still setassetdata on a leased asset", async () => { await mint(); await leaseFor(); @@ -280,12 +244,18 @@ describe("non-custodial rental primitives", () => { third.name.toString() ]).send(`${lister.name.toString()}@active`)).rejects.toThrow("missing required authority"); - // Re-point the market to `third`, who can now open leases. + // Re-point the market to `third`, who can now open leases; the default + // market ("atomicmarket") can no longer. await atomicassets.actions.setrentmkt([ third.name.toString() ]).send(`${atomicassets.name.toString()}@active`); const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + market.name.toString(), lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "lease" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("not the configured rental market"); + await expect(atomicassets.actions.leasestart([ third.name.toString(), lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "lease" @@ -320,52 +290,40 @@ describe("non-custodial rental primitives", () => { expect(leases()).toEqual([]); // lock cleared }); - test("delpretitle clears a sentinel; reclaim refuses a sentinel", async () => { + test("reclaim throws when the asset is not leased", async () => { await mint(); - await atomicassets.actions.pretitle([ - lister.name.toString(), market.name.toString(), ASSET1 - ]).send(`${lister.name.toString()}@active`); - - // reclaim is for expired leases, not sentinels await expect(atomicassets.actions.reclaim([ ASSET1 - ]).send(`${third.name.toString()}@active`)).rejects.toThrow("not leased; use delpretitle"); - - await atomicassets.actions.delpretitle([ - ASSET1 - ]).send(`${lister.name.toString()}@active`); - expect(leases()).toEqual([]); - // unlocked again - await expect(atomicassets.actions.transfer([ - lister.name.toString(), renter.name.toString(), [ASSET1], "" - ]).send(`${lister.name.toString()}@active`)).resolves.not.toThrow(); + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("not leased"); }); - // ------------------------------------------------------------ offer clearing + // ------------------------------------------------ offers survive a rental - test("leasestart clears a stale offer the lister created for the asset", async () => { + test("a pre-existing offer survives a rental and is acceptable again after reclaim", async () => { await mint(); - // lister lists the asset in an out-offer BEFORE leasing + // lister offers the asset to `third` BEFORE leasing it await atomicassets.actions.createoffer([ lister.name.toString(), third.name.toString(), [ASSET1], [], "" ]).send(`${lister.name.toString()}@active`); expect(offers()).toHaveLength(1); - await leaseFor(); - - // the stale offer is gone, so it can never settle around the lock - expect(offers()).toEqual([]); - }); - - test("reclaim clears a stale offer the renter created for the asset", async () => { - await mint(); await leaseFor(ONE_HOUR); - // Force a stale renter offer into the table directly is not possible - // (createoffer is guarded), so this asserts the table stays clean across - // a reclaim — the renter could never have created one. + // the offer is NOT cleared by lease-start; it just can't settle while the + // asset is locked / owned by the renter + expect(offers()).toHaveLength(1); + await expect(atomicassets.actions.acceptoffer([ + 1 + ]).send(`${third.name.toString()}@active`)).rejects.toThrow(); + + // after reclaim the asset is back with the lister and unlocked, so the same + // offer can now be accepted blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); await atomicassets.actions.reclaim([ASSET1]).send(`${third.name.toString()}@active`); - expect(offers()).toEqual([]); + + await expect(atomicassets.actions.acceptoffer([ + 1 + ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); + expect(assetsOf(third).map((a) => a.asset_id)).toContain(ASSET1); }); }); From 5a89ff1562c8215a6e6f1af39892ac19c62a7baf Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 22:02:15 -0400 Subject: [PATCH 03/11] fix(rentals): reject extending an already-expired lease (Copilot review) leaseextend only required the new end to be later than the current one, so once a lease had expired the configured market could race the permissionless reclaim and push rental_end into the future, indefinitely blocking the guaranteed revert to the title_owner. Require the lease to still be active (now < rental_end); an expired lease can only be reclaimed. AtomicMarket already only extends an active lease, so this does not affect the normal flow. Adds a VeRT regression. --- src/atomicassets.cpp | 8 ++++++++ tests/asset-actions/renting-invariants.test.js | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index 9d10b63..e53635f 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -180,6 +180,14 @@ ACTION atomicassets::leaseextend( leases_t leases = get_leases(); auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); + + // An expired lease can only be reclaimed, never extended. Otherwise the + // configured market could race the permissionless reclaim after expiry and + // push rental_end into the future, indefinitely blocking the guaranteed + // revert to the title_owner. + uint32_t now = eosio::current_time_point().sec_since_epoch(); + check(now < lease_itr->rental_end, "Lease has already expired; it must be reclaimed, not extended"); + check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); name title_owner = lease_itr->title_owner; diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index 0b9fb0b..f188f60 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -180,6 +180,21 @@ describe("non-custodial rental primitives", () => { expect(leases()[0].rental_end).toBe(newEnd); }); + test("leaseextend cannot revive an expired lease (no racing the reclaim)", async () => { + await mint(); + await leaseFor(ONE_HOUR); + + // jump past expiry, then the market tries to push rental_end out + blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); + await expect(atomicassets.actions.leaseextend([ + market.name.toString(), ASSET1, nowSec() + ONE_HOUR + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("already expired"); + + // reclaim is still available and returns the asset to the lister + await atomicassets.actions.reclaim([ASSET1]).send(`${third.name.toString()}@active`); + expect(assetsOf(lister).map((a) => a.asset_id)).toContain(ASSET1); + }); + // -------------------------------------------------------------- lock guards test("a leased asset cannot be transferred by the renter (its owner)", async () => { From b4043888a044786c5bb3adad4c12ab2ae73f84cc Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 22:23:50 -0400 Subject: [PATCH 04/11] feat(rentals): record lease start so the duration cap can be total, not rolling Add rental_start to the leases table, set when the lease is first opened (leasestart) and left unchanged across extensions (leaseextend). This lets AtomicMarket cap the total rental period from the original start rather than a rolling window from "now". Action signatures are unchanged; only the leases table gains a field. --- include/atomicassets-interface.hpp | 1 + include/atomicassets.hpp | 1 + src/atomicassets.cpp | 11 ++++++----- tests/asset-actions/renting-invariants.test.js | 1 + 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index d8d1e2e..421a2e3 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -152,6 +152,7 @@ namespace atomicassets { uint64_t asset_id; name title_owner; name renter; + uint32_t rental_start; uint32_t rental_end; name market; diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index fa49705..d2e121f 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -468,6 +468,7 @@ CONTRACT atomicassets : public contract { uint64_t asset_id; name title_owner; // lister; reclaim returns the asset here name renter; // current AA owner during the lease + uint32_t rental_start; // sec_since_epoch the lease was first opened (fixed across extensions) uint32_t rental_end; // sec_since_epoch the lease expires name market; // rental market that opened/manages the lease diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index e53635f..2623043 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -144,11 +144,12 @@ ACTION atomicassets::leasestart( // Write the lock row FIRST so there is no instant where the asset is // renter-owned but unlocked. leases.emplace(market, [&](auto &_lease) { - _lease.asset_id = asset_id; - _lease.title_owner = title_owner; - _lease.renter = renter; - _lease.rental_end = rental_end; - _lease.market = market; + _lease.asset_id = asset_id; + _lease.title_owner = title_owner; + _lease.renter = renter; + _lease.rental_start = now; + _lease.rental_end = rental_end; + _lease.market = market; }); // Flip ownership lister -> renter under the contract's own authority. The diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index f188f60..47d651c 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -138,6 +138,7 @@ describe("non-custodial rental primitives", () => { asset_id: ASSET1, title_owner: lister.name.toString(), renter: renter.name.toString(), + rental_start: rentalEnd - ONE_HOUR, rental_end: rentalEnd, market: market.name.toString() }]); From 5b2c9872c0d65d2afd287c0f13d5cb122e2fe542 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 22:44:57 -0400 Subject: [PATCH 05/11] refactor(rentals): drop dead sentinel branches in loglock/logreclaim A leases row always represents an active lease now (pretitle/sentinels were removed), so renter is never empty. Drop the `if (renter != name(""))` guards and always notify the renter. --- src/atomicassets.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index 2623043..f9b97f9 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -1639,9 +1639,7 @@ ACTION atomicassets::loglock( require_auth(get_self()); require_recipient(title_owner); - if (renter != name("")) { - require_recipient(renter); - } + require_recipient(renter); notify_collection_accounts(collection_name); } @@ -1654,9 +1652,7 @@ ACTION atomicassets::logreclaim( require_auth(get_self()); require_recipient(title_owner); - if (renter != name("")) { - require_recipient(renter); - } + require_recipient(renter); notify_collection_accounts(collection_name); } From 1f5f5f01edcf929186eb1b120b6a6e6eae032947 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 23:04:42 -0400 Subject: [PATCH 06/11] refactor(rentals): drop the market param and leases.market field With a single configured rental market, passing and storing a market identity is redundant. leasestart/leaseextend no longer take a market argument; authority is simply require_auth(rentalcfg.rental_market) via check_rental_market(), which returns the configured account for use as the lease row's RAM payer. Drop the leases.market column and the market field from loglock. Tests updated; the authority tests now assert the missing-required-authority path. --- include/atomicassets-interface.hpp | 1 - include/atomicassets.hpp | 11 +++---- src/atomicassets.cpp | 30 ++++++++----------- .../asset-actions/renting-invariants.test.js | 26 ++++++++-------- 4 files changed, 30 insertions(+), 38 deletions(-) diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index 421a2e3..b800311 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -154,7 +154,6 @@ namespace atomicassets { name renter; uint32_t rental_start; uint32_t rental_end; - name market; uint64_t primary_key() const { return asset_id; }; uint64_t by_title_owner() const { return title_owner.value; }; diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index d2e121f..d3932a5 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -39,7 +39,6 @@ CONTRACT atomicassets : public contract { ); ACTION leasestart( - name market, name title_owner, name renter, uint64_t asset_id, @@ -48,7 +47,6 @@ CONTRACT atomicassets : public contract { ); ACTION leaseextend( - name market, uint64_t asset_id, uint32_t rental_end ); @@ -280,8 +278,7 @@ CONTRACT atomicassets : public contract { uint64_t asset_id, name title_owner, name renter, - uint32_t rental_end, - name market + uint32_t rental_end ); ACTION logreclaim( @@ -470,7 +467,6 @@ CONTRACT atomicassets : public contract { name renter; // current AA owner during the lease uint32_t rental_start; // sec_since_epoch the lease was first opened (fixed across extensions) uint32_t rental_end; // sec_since_epoch the lease expires - name market; // rental market that opened/manages the lease uint64_t primary_key() const { return asset_id; }; uint64_t by_title_owner() const { return title_owner.value; }; @@ -607,8 +603,9 @@ CONTRACT atomicassets : public contract { // Reverts if the asset has a live lease/title record (i.e. is rental-locked). void check_not_leased(uint64_t asset_id); - // Asserts `market` is the configured rental market and requires its auth. - void check_rental_market(name market); + // Requires the authorization of the configured rental market (the single + // account allowed to open/manage leases) and returns it. + name check_rental_market(); void notify_collection_accounts( name collection_name diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index f9b97f9..4f14730 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -107,17 +107,16 @@ ACTION atomicassets::setrentmkt(name rental_market) { * unlocked window (the lease row is written before the ownership flip). The * configured rental market is trusted to have verified the lister's consent (on * AtomicMarket the lister's announcerent carries that authorization). -* @required_auth market (must be the configured rental_market) +* @required_auth the configured rental market */ ACTION atomicassets::leasestart( - name market, name title_owner, name renter, uint64_t asset_id, uint32_t rental_end, string memo ) { - check_rental_market(market); + name market = check_rental_market(); check(is_account(renter), "renter account does not exist"); check(renter != title_owner, "renter and title_owner cannot be the same"); @@ -149,7 +148,6 @@ ACTION atomicassets::leasestart( _lease.renter = renter; _lease.rental_start = now; _lease.rental_end = rental_end; - _lease.market = market; }); // Flip ownership lister -> renter under the contract's own authority. The @@ -163,21 +161,20 @@ ACTION atomicassets::leasestart( permission_level{get_self(), name("active")}, get_self(), name("loglock"), - make_tuple(collection_name, asset_id, title_owner, renter, rental_end, market) + make_tuple(collection_name, asset_id, title_owner, renter, rental_end) ).send(); } /** * Extends an active lease's end time. Does not change ownership. -* @required_auth market (must be the configured rental_market) +* @required_auth the configured rental market */ ACTION atomicassets::leaseextend( - name market, uint64_t asset_id, uint32_t rental_end ) { - check_rental_market(market); + name market = check_rental_market(); leases_t leases = get_leases(); auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); @@ -206,7 +203,7 @@ ACTION atomicassets::leaseextend( permission_level{get_self(), name("active")}, get_self(), name("loglock"), - make_tuple(collection_name, asset_id, title_owner, renter, rental_end, market) + make_tuple(collection_name, asset_id, title_owner, renter, rental_end) ).send(); } @@ -1633,8 +1630,7 @@ ACTION atomicassets::loglock( uint64_t asset_id, name title_owner, name renter, - uint32_t rental_end, - name market + uint32_t rental_end ) { require_auth(get_self()); @@ -1982,14 +1978,14 @@ void atomicassets::check_not_leased(uint64_t asset_id) { /** -* Asserts that `market` is the configured rental market and is authorized. The -* configured account is the single trusted opener/manager of leases. +* Requires the authorization of the configured rental market (the single account +* trusted to open and manage leases) and returns it. name("") disables leasing. */ -void atomicassets::check_rental_market(name market) { +name atomicassets::check_rental_market() { name configured = get_rentalcfg().get_or_default(rentalcfg_s{}).rental_market; - check(configured != name("") && market == configured, - "market is not the configured rental market"); - require_auth(market); + check(configured != name(""), "Leasing is disabled (no rental market configured)"); + require_auth(configured); + return configured; } diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index 47d651c..9109c8b 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -113,7 +113,6 @@ describe("non-custodial rental primitives", () => { async function leaseFor(seconds = ONE_HOUR) { const rentalEnd = nowSec() + seconds; await atomicassets.actions.leasestart([ - market.name.toString(), lister.name.toString(), renter.name.toString(), ASSET1, @@ -139,8 +138,7 @@ describe("non-custodial rental primitives", () => { title_owner: lister.name.toString(), renter: renter.name.toString(), rental_start: rentalEnd - ONE_HOUR, - rental_end: rentalEnd, - market: market.name.toString() + rental_end: rentalEnd }]); }); @@ -155,7 +153,7 @@ describe("non-custodial rental primitives", () => { await leaseFor(); const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ - market.name.toString(), lister.name.toString(), renter.name.toString(), + lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "second lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("already leased"); }); @@ -164,7 +162,7 @@ describe("non-custodial rental primitives", () => { await mint(2); // non-transferable template const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ - market.name.toString(), lister.name.toString(), renter.name.toString(), + lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("not transferable"); }); @@ -174,7 +172,7 @@ describe("non-custodial rental primitives", () => { await leaseFor(); const newEnd = nowSec() + ONE_HOUR * 5; await atomicassets.actions.leaseextend([ - market.name.toString(), ASSET1, newEnd + ASSET1, newEnd ]).send(`${market.name.toString()}@active`); expect(assetsOf(renter)).toHaveLength(1); // still the renter's @@ -188,7 +186,7 @@ describe("non-custodial rental primitives", () => { // jump past expiry, then the market tries to push rental_end out blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); await expect(atomicassets.actions.leaseextend([ - market.name.toString(), ASSET1, nowSec() + ONE_HOUR + ASSET1, nowSec() + ONE_HOUR ]).send(`${market.name.toString()}@active`)).rejects.toThrow("already expired"); // reclaim is still available and returns the asset to the lister @@ -245,13 +243,15 @@ describe("non-custodial rental primitives", () => { // ----------------------------------------------------------------- authority - test("throw when an unconfigured account tries to open a lease", async () => { + test("throw when an account other than the configured market opens a lease", async () => { await mint(); const rentalEnd = nowSec() + ONE_HOUR; + // signed by `third`, not the configured market (atomicmarket), so the + // required authority of the configured market is missing await expect(atomicassets.actions.leasestart([ - third.name.toString(), lister.name.toString(), renter.name.toString(), + lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "lease" - ]).send(`${third.name.toString()}@active`)).rejects.toThrow("not the configured rental market"); + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("missing required authority"); }); test("setrentmkt requires contract authority and reconfigures the market", async () => { @@ -268,12 +268,12 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ - market.name.toString(), lister.name.toString(), renter.name.toString(), + lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "lease" - ]).send(`${market.name.toString()}@active`)).rejects.toThrow("not the configured rental market"); + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("missing required authority"); await expect(atomicassets.actions.leasestart([ - third.name.toString(), lister.name.toString(), renter.name.toString(), + lister.name.toString(), renter.name.toString(), ASSET1, rentalEnd, "lease" ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); expect(assetsOf(renter)).toHaveLength(1); From ad8fb43b8faf6e57ccfde03a6063563f2feb81ea Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Sun, 28 Jun 2026 23:17:45 -0400 Subject: [PATCH 07/11] docs(rentals): clarify scope_payer=get_self() RAM billing in internal_transfer (Copilot review) --- src/atomicassets.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index 4f14730..b7c63ed 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -1922,7 +1922,10 @@ void atomicassets::internal_transfer( if (no_previous_scope) { //A dummy asset is emplaced, which makes the scope_payer pay for the ram of the scope //This asset is later deleted again. - //This action will therefore fail is the scope_payer didn't authorize the action + //This requires scope_payer to have authorized the action - EXCEPT when scope_payer is + //the contract itself (get_self()), which can always bill its own RAM. The privileged + //rental paths (leasestart, permissionless reclaim) rely on that: they pass get_self() + //so no title_owner/renter signature is needed to create the destination scope. to_assets.emplace(scope_payer, [&](auto &_asset) { _asset.asset_id = ULLONG_MAX; _asset.collection_name = name(""); From 1e6579d72340560fa935589bbf6fbcd06066251e Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Mon, 29 Jun 2026 16:34:45 -0400 Subject: [PATCH 08/11] fix(rentals): make permissionless reclaim un-vetoable by the renter The non-custodial model rests on reclaim always returning the asset to the lister at expiry. logreclaim notified the renter via require_recipient, so a renter that is a contract could throw in its handler and abort every reclaim, keeping the asset forever (the old `move` already warned that from/to notifications are "exploitable"). Drop the renter and title_owner recipients from logreclaim. The asset's collection is still notified (on logtransfer and logreclaim), trusting collections not to grief their own collection. Also add a MAX_LEASE_SECONDS (28d) protocol backstop in leasestart/leaseextend so a compromised or buggy rental market can't mint a near-permanent lock; the market keeps its own product cap on top. Tests: new evil-renter fixture proves the renter cannot veto reclaim while the collection still is; plus duration-cap rejections. --- Makefile | 11 ++- include/atomicassets.hpp | 7 ++ src/atomicassets.cpp | 33 +++++-- .../asset-actions/renting-invariants.test.js | 88 +++++++++++++++++++ tests/fixtures/evil-renter/evil-renter.cpp | 45 ++++++++++ 5 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/evil-renter/evil-renter.cpp diff --git a/Makefile b/Makefile index 07d5107..35438b7 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ build: mkdir -p build cdt-cpp -abigen -contract=atomicassets -I./include src/atomicassets.cpp -o build/atomicassets.wasm $(MAKE) build-test-consumer + $(MAKE) build-evil-renter # Test-only fixture: a minimal EXTERNAL contract that reads atomicassets tables # through include/atomicassets-interface.hpp. Catches header regressions the @@ -12,6 +13,14 @@ build-test-consumer: mkdir -p build cdt-cpp -abigen -contract=ifaceconsumr -I./include tests/fixtures/interface-consumer/interface-consumer.cpp -o build/interface-consumer.wasm +# Test-only adversary: a renter/collection contract that throws on the +# atomicassets::logreclaim notification. Proves the permissionless reclaim can no +# longer be vetoed by a hostile notification handler (the asset-trap fix). +# Consumed by tests/asset-actions/renting-invariants.test.js; NOT a release artifact. +build-evil-renter: + mkdir -p build + cdt-cpp -abigen -contract=evilrenter -I./include tests/fixtures/evil-renter/evil-renter.cpp -o build/evil-renter.wasm + # Release-only ABI normalization. CDT 4.1 changed two -abigen spellings # (pair fields first/second; vector as `bytes`) that break existing # integrations. The VeRT test suite is written against the raw CDT 4.1 abi, so we @@ -33,6 +42,6 @@ export-memory: wat2wasm -o build/atomicassets.wasm atomicassets.wat rm atomicassets.wat -.PHONY: build build-test-consumer patch-abi release export-memory clean +.PHONY: build build-test-consumer build-evil-renter patch-abi release export-memory clean clean: -rm -rf build \ No newline at end of file diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index d3932a5..9cc856f 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -13,6 +13,13 @@ using namespace atomicdata; static constexpr double MAX_MARKET_FEE = 0.15; static constexpr uint32_t AUTHOR_SWAP_TIME_DELTA = 60 * 60 * 24 * 7; // 1 week, valid for 1 week +// Defense-in-depth backstop on the non-custodial rental primitive: the protocol caps any single +// lease (and any extension, measured from the lease's fixed rental_start) at this duration so a +// compromised or buggy configured rental_market cannot mint a near-permanent (~year 2106) lock on +// an asset. The product-level limit lives in the rental market (AtomicMarket enforces the same +// 28 days); this is the hard protocol ceiling regardless of which market is configured. +static constexpr uint32_t MAX_LEASE_SECONDS = 60 * 60 * 24 * 28; // 28 days + static constexpr char COLLECTION_NOT_FOUND[] = "No collection with this name exists"; CONTRACT atomicassets : public contract { diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index b7c63ed..bc74c5a 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -124,6 +124,10 @@ ACTION atomicassets::leasestart( uint32_t now = eosio::current_time_point().sec_since_epoch(); check(rental_end > now, "rental_end must be in the future"); + // Protocol backstop: cap the lease duration so a compromised/buggy rental market cannot + // mint a near-permanent lock. The market enforces its own (tighter) product limit on top. + check(rental_end - now <= MAX_LEASE_SECONDS, "rental_end exceeds the maximum lease duration"); + leases_t leases = get_leases(); check(leases.find(asset_id) == leases.end(), "Asset is already leased"); @@ -188,6 +192,11 @@ ACTION atomicassets::leaseextend( check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); + // Protocol backstop: cap the TOTAL lease window from the fixed rental_start (not from "now"), + // so repeated extensions can't roll the asset forward indefinitely past the maximum. + check(rental_end - lease_itr->rental_start <= MAX_LEASE_SECONDS, + "rental_end exceeds the maximum lease duration"); + name title_owner = lease_itr->title_owner; name renter = lease_itr->renter; @@ -234,9 +243,14 @@ ACTION atomicassets::reclaim( // Erase the lock, then move the asset back under the contract's own authority // (enforce_lock=false). The contract pays any transient scope RAM so reclaim - // never needs the title_owner's or renter's signature. A pre-existing offer - // referencing the asset is left in place (it could not settle while locked, - // and becomes valid again now that the asset is back with its owner). + // never needs the title_owner's or renter's signature. The move emits the normal + // logtransfer (notifying the asset's collection), and logreclaim below notifies + // the collection of the structured reclaim event. Crucially, NEITHER notifies the + // renter or title_owner: a renter is an arbitrary (possibly hostile) account, and + // notifying it would let it abort this guaranteed revert by throwing in a handler + // and trap the asset forever. A pre-existing offer referencing the asset is left + // in place (it could not settle while locked, and becomes valid again now that the + // asset is back with its owner). leases.erase(lease_itr); internal_transfer(renter, title_owner, vector{asset_id}, "lease reclaim", get_self(), false); @@ -1647,8 +1661,17 @@ ACTION atomicassets::logreclaim( ) { require_auth(get_self()); - require_recipient(title_owner); - require_recipient(renter); + // The asset's collection is notified of the reclaim (mirrors loglock on lease-start), so a + // collection can react to its assets returning. This trusts collections not to grief their + // own collection: a collection notify-account that throws here CAN abort the reclaim and trap + // the asset, which is accepted under the same trust model that lets collections gate transfers. + // + // Deliberately NO require_recipient(renter) / require_recipient(title_owner): reclaim is the + // permissionless guaranteed revert the whole model rests on. The renter is an arbitrary, + // possibly hostile account that profits from keeping the asset; notifying it would hand it a + // veto (throw in a handler -> abort the reclaim -> asset trapped forever). The title_owner is + // the beneficiary and learns of the reclaim by receiving the asset and this trace, so there is + // no reason to give it an abort lever either. notify_collection_accounts(collection_name); } diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index 9109c8b..af005d9 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -18,9 +18,11 @@ describe("non-custodial rental primitives", () => { let lister; // title_owner / lessor let renter; // becomes the AA owner during the lease let third; // unrelated third party / random reclaim caller + let evil; // adversary contract: throws on the atomicassets::logreclaim notification const ASSET1 = "1099511627776"; // 2^40, first minted asset id const ONE_HOUR = 3600; + const MAX_LEASE_SECONDS = 60 * 60 * 24 * 28; // mirrors the contract's protocol backstop function nowSec() { return Math.floor(blockchain.timestamp.toMilliseconds() / 1000); @@ -44,6 +46,8 @@ describe("non-custodial rental primitives", () => { lister = blockchain.createAccount('lister'); renter = blockchain.createAccount('renter'); third = blockchain.createAccount('thirduser11'); + // Adversary contract that vetoes the reclaim notification (see fixtures/evil-renter). + evil = blockchain.createContract('evilrenter11', './build/evil-renter'); }); beforeEach(async () => { @@ -342,4 +346,88 @@ describe("non-custodial rental primitives", () => { ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); expect(assetsOf(third).map((a) => a.asset_id)).toContain(ASSET1); }); + + // ----------------------------------- the RENTER cannot veto the permissionless reclaim + // The permissionless reclaim is the model's guaranteed revert. The renter is an + // arbitrary, possibly hostile account that profits from keeping the asset, so it + // must NEVER be able to abort the reclaim by throwing in a notification handler. + // The `evil` fixture throws on the atomicassets::logreclaim notification; reclaim + // no longer notifies the renter, so the veto can't fire. + // (Re-adding require_recipient(renter) to logreclaim makes this test RED.) + + test("a malicious renter contract cannot veto the permissionless reclaim", async () => { + await mint(); + // lease to the EVIL contract account; it becomes the real owner. loglock is + // delivered at lease-start but evil only vetoes logreclaim, so this succeeds. + const rentalEnd = nowSec() + ONE_HOUR; + await atomicassets.actions.leasestart([ + lister.name.toString(), evil.name.toString(), + ASSET1, rentalEnd, "lease to evil renter" + ]).send(`${market.name.toString()}@active`); + expect(assetsOf(evil)).toHaveLength(1); + + blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); + + // the renter is no longer notified on reclaim, so its veto never fires + await expect(atomicassets.actions.reclaim([ + ASSET1 + ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); + + expect(assetsOf(evil)).toHaveLength(0); + expect(assetsOf(lister).map((a) => a.asset_id)).toContain(ASSET1); + expect(leases()).toEqual([]); // lock cleared, asset returned + }); + + test("the collection IS notified on reclaim (trusted; can react, by design)", async () => { + // By design reclaim notifies the asset's collection (mirrors loglock on + // lease-start) so collections can react to their assets returning. This is a + // deliberate trust tradeoff: a collection notify-account that throws CAN abort + // the reclaim — accepted under the same trust model that lets collections gate + // transfers, and in pointed contrast to the renter, which cannot (test above). + // We prove the collection is actually reached by making its notify-account the + // `evil` fixture (throws on logreclaim) and observing the reclaim revert. + await mint(); + await atomicassets.actions.addnotifyacc([ + "testcollect1", evil.name.toString() + ]).send(`${lister.name.toString()}@active`); + + await leaseFor(ONE_HOUR); // ordinary renter + blockchain.addTime(TimePoint.fromMilliseconds((ONE_HOUR + 1) * 1000)); + + // the collection notify-account is in the reclaim path, so its veto reverts it + await expect(atomicassets.actions.reclaim([ + ASSET1 + ]).send(`${third.name.toString()}@active`)).rejects.toThrow("evil renter vetoes the reclaim"); + }); + + // ----------------------------------------- duration cap (protocol backstop, #2) + + test("leasestart rejects a rental_end beyond MAX_LEASE_SECONDS", async () => { + await mint(); + const tooLong = nowSec() + MAX_LEASE_SECONDS + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + lister.name.toString(), renter.name.toString(), + ASSET1, tooLong, "too long" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); + }); + + test("leasestart allows a rental_end exactly at MAX_LEASE_SECONDS", async () => { + await mint(); + const atCap = nowSec() + MAX_LEASE_SECONDS; + await expect(atomicassets.actions.leasestart([ + lister.name.toString(), renter.name.toString(), + ASSET1, atCap, "at cap" + ]).send(`${market.name.toString()}@active`)).resolves.not.toThrow(); + expect(assetsOf(renter)).toHaveLength(1); + }); + + test("leaseextend cannot push the total window past MAX_LEASE_SECONDS from rental_start", async () => { + await mint(); + const rentalStart = nowSec(); + await leaseFor(ONE_HOUR); + // total window measured from the fixed rental_start, not from "now" + await expect(atomicassets.actions.leaseextend([ + ASSET1, rentalStart + MAX_LEASE_SECONDS + ONE_HOUR + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); + }); }); diff --git a/tests/fixtures/evil-renter/evil-renter.cpp b/tests/fixtures/evil-renter/evil-renter.cpp new file mode 100644 index 0000000..81c2558 --- /dev/null +++ b/tests/fixtures/evil-renter/evil-renter.cpp @@ -0,0 +1,45 @@ +/* + Test-only adversary contract for the non-custodial rental reclaim path. + + The permissionless `reclaim` is the guaranteed revert the whole rental model + rests on: at expiry anyone can return a leased asset to its title_owner, with + no renter signature. The hazard (the original `move` action warned of it: "Cannot + have notifications for the from & to, exploitable") is that a renter which is a + CONTRACT can veto the reclaim by throwing inside a notification handler, since a + throwing `require_recipient` target aborts the entire transaction. That would + trap the asset with the renter forever, defeating the model. + + This contract is exactly such an adversary: it aborts ONLY when notified of + `atomicassets::logreclaim`, and ignores every other notification (notably + `loglock`, so it can still receive the asset at lease start). It pins down two + things about the reclaim path: + - Deployed as the RENTER, it proves the renter is NOT notified on reclaim + (reclaim succeeds despite the veto) — the asset-trap theft vector is closed. + - Deployed as a COLLECTION notify-account, it proves the collection IS notified + on reclaim (reclaim reverts) — a deliberate trust tradeoff: collections can + react to (and, if hostile, block) reclaim of their own collection's assets. + + Built by `make build` into build/evil-renter.{wasm,abi}; consumed by + tests/asset-actions/renting-invariants.test.js. NOT a distributable artifact. +*/ + +#include + +using namespace eosio; + +CONTRACT evilrenter : public contract { +public: + using contract::contract; + + // The veto: abort whenever atomicassets emits the reclaim log to us. Pre-fix + // (logreclaim require_recipient(renter)) this aborts every reclaim attempt and + // traps the asset. Post-fix this handler is never invoked. + [[eosio::on_notify("atomicassets::logreclaim")]] + void onreclaim(name collection_name, uint64_t asset_id, name title_owner, name renter) { + check(false, "evil renter vetoes the reclaim"); + } + + // No-op action; exists only so -abigen emits an ABI (a notification-only + // contract is "empty" to abigen and produces none, which VeRT needs to load). + ACTION noop() {} +}; From fcd509c2834b215b68f017c6c8b2a4befc5770f1 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Tue, 30 Jun 2026 15:50:04 -0400 Subject: [PATCH 09/11] refactor(rentals): opt-in leasing, denormalize collection, trim comments Post-review cleanup of the non-custodial lease primitives (pairs with atomicmarket-contract#11): - Leasing is now opt-in: rentalcfg defaults to name("") (disabled) instead of atomicmarket, so a fresh deploy is off until governance calls setrentmkt; setrentmkt(name("")) stays the kill-switch. - Denormalize collection_name onto the leases row, removing the unreachable "renter no longer owns the asset" refetch in leaseextend/reclaim. - Extract send_loglock to drop the duplicated loglock emit. - Trim the verbose rental comments to the house style: consolidate the reclaim-veto rationale into logreclaim, drop em-dashes and the instructional error string, reuse the collection_name local. --- include/atomicassets-interface.hpp | 1 + include/atomicassets.hpp | 22 +-- src/atomicassets.cpp | 139 +++++++----------- .../asset-actions/renting-invariants.test.js | 23 ++- 4 files changed, 89 insertions(+), 96 deletions(-) diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index b800311..4875888 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -152,6 +152,7 @@ namespace atomicassets { uint64_t asset_id; name title_owner; name renter; + name collection_name; uint32_t rental_start; uint32_t rental_end; diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index 9cc856f..c6f885d 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -13,11 +13,8 @@ using namespace atomicdata; static constexpr double MAX_MARKET_FEE = 0.15; static constexpr uint32_t AUTHOR_SWAP_TIME_DELTA = 60 * 60 * 24 * 7; // 1 week, valid for 1 week -// Defense-in-depth backstop on the non-custodial rental primitive: the protocol caps any single -// lease (and any extension, measured from the lease's fixed rental_start) at this duration so a -// compromised or buggy configured rental_market cannot mint a near-permanent (~year 2106) lock on -// an asset. The product-level limit lives in the rental market (AtomicMarket enforces the same -// 28 days); this is the hard protocol ceiling regardless of which market is configured. +// Protocol ceiling on a lease (and its total extended window, from the fixed rental_start), so a +// compromised or buggy rental_market can't mint a near-permanent lock. AtomicMarket caps to the same. static constexpr uint32_t MAX_LEASE_SECONDS = 60 * 60 * 24 * 28; // 28 days static constexpr char COLLECTION_NOT_FOUND[] = "No collection with this name exists"; @@ -472,6 +469,7 @@ CONTRACT atomicassets : public contract { uint64_t asset_id; name title_owner; // lister; reclaim returns the asset here name renter; // current AA owner during the lease + name collection_name; uint32_t rental_start; // sec_since_epoch the lease was first opened (fixed across extensions) uint32_t rental_end; // sec_since_epoch the lease expires @@ -525,13 +523,12 @@ CONTRACT atomicassets : public contract { typedef singleton config_t; - // The single account authorized to open/manage non-custodial rental leases - // (leasestart/leaseextend). Kept in its OWN singleton (not appended to config) - // so deploying onto an existing contract needs no config migration: an absent - // row reads as the default (the AtomicMarket contract). setrentmkt overwrites - // it; name("") disables leasing. + // The single account authorized to open/manage leases (leasestart/leaseextend), in its own + // singleton so it needs no config migration. Leasing is opt-in: name("") (the default, and an + // absent row) means disabled, so a fresh deploy is off until setrentmkt("atomicmarket"); set it + // back to name("") to kill-switch all leasing. Not hardcoded - the market account differs per chain. TABLE rentalcfg_s { - name rental_market = name("atomicmarket"); + name rental_market = name(""); }; typedef singleton rentalcfg_t; @@ -614,6 +611,9 @@ CONTRACT atomicassets : public contract { // account allowed to open/manage leases) and returns it. name check_rental_market(); + // Emits the loglock action (shared by leasestart and leaseextend). + void send_loglock(name collection_name, uint64_t asset_id, name title_owner, name renter, uint32_t rental_end); + void notify_collection_accounts( name collection_name ); diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index bc74c5a..ddc2d3a 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -102,11 +102,9 @@ ACTION atomicassets::setrentmkt(name rental_market) { /** -* Opens a non-custodial rental lease: makes `renter` the real AtomicAssets owner -* of the asset and parks the lister's reclaim right in the lease record, with NO -* unlocked window (the lease row is written before the ownership flip). The -* configured rental market is trusted to have verified the lister's consent (on -* AtomicMarket the lister's announcerent carries that authorization). +* Opens a non-custodial lease: `renter` becomes the real owner, the lister's reclaim right is +* parked in the lease row, and the asset is locked. The configured market is trusted to have +* verified the lister's consent (AtomicMarket's announcerent carries it). * @required_auth the configured rental market */ ACTION atomicassets::leasestart( @@ -124,8 +122,7 @@ ACTION atomicassets::leasestart( uint32_t now = eosio::current_time_point().sec_since_epoch(); check(rental_end > now, "rental_end must be in the future"); - // Protocol backstop: cap the lease duration so a compromised/buggy rental market cannot - // mint a near-permanent lock. The market enforces its own (tighter) product limit on top. + // Protocol backstop against a compromised/buggy market minting a near-permanent lock. check(rental_end - now <= MAX_LEASE_SECONDS, "rental_end exceeds the maximum lease duration"); leases_t leases = get_leases(); @@ -136,37 +133,28 @@ ACTION atomicassets::leasestart( "title_owner does not own this asset"); name collection_name = asset_itr->collection_name; - // A non-transferable asset can never be leased out (fail early with a clear - // message; internal_transfer would otherwise reject it after the row write). + // internal_transfer re-checks this; fail early with a clear message. if (asset_itr->template_id >= 0) { - templates_t collection_templates = get_templates(asset_itr->collection_name); + templates_t collection_templates = get_templates(collection_name); auto template_itr = collection_templates.find(asset_itr->template_id); check(template_itr->transferable, "The asset is not transferable"); } - // Write the lock row FIRST so there is no instant where the asset is - // renter-owned but unlocked. + // Write the lock row before the ownership flip (no renter-owned-but-unlocked instant). leases.emplace(market, [&](auto &_lease) { - _lease.asset_id = asset_id; - _lease.title_owner = title_owner; - _lease.renter = renter; - _lease.rental_start = now; - _lease.rental_end = rental_end; + _lease.asset_id = asset_id; + _lease.title_owner = title_owner; + _lease.renter = renter; + _lease.collection_name = collection_name; + _lease.rental_start = now; + _lease.rental_end = rental_end; }); - // Flip ownership lister -> renter under the contract's own authority. The - // lock is already in force, so this is the privileged (enforce_lock=false) - // path. The contract pays any transient scope RAM. A pre-existing offer that - // references the asset is intentionally left in place: it cannot settle while - // the asset is locked, and becomes valid again once the asset is reclaimed. + // Flip lister -> renter under the contract's own authority: the lock is in force, so this is + // the privileged enforce_lock=false path, and the contract pays any transient scope RAM. internal_transfer(title_owner, renter, vector{asset_id}, memo, get_self(), false); - action( - permission_level{get_self(), name("active")}, - get_self(), - name("loglock"), - make_tuple(collection_name, asset_id, title_owner, renter, rental_end) - ).send(); + send_loglock(collection_name, asset_id, title_owner, renter, rental_end); } @@ -183,46 +171,33 @@ ACTION atomicassets::leaseextend( leases_t leases = get_leases(); auto lease_itr = leases.require_find(asset_id, "Asset is not leased"); - // An expired lease can only be reclaimed, never extended. Otherwise the - // configured market could race the permissionless reclaim after expiry and - // push rental_end into the future, indefinitely blocking the guaranteed - // revert to the title_owner. + // Expired leases can only be reclaimed, not extended - else the market could race the + // permissionless reclaim and push rental_end out, blocking the guaranteed revert. uint32_t now = eosio::current_time_point().sec_since_epoch(); - check(now < lease_itr->rental_end, "Lease has already expired; it must be reclaimed, not extended"); + check(now < lease_itr->rental_end, "Lease has already expired"); check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); - // Protocol backstop: cap the TOTAL lease window from the fixed rental_start (not from "now"), - // so repeated extensions can't roll the asset forward indefinitely past the maximum. + // Cap the total window from the fixed rental_start, so repeated extensions can't roll past the max. check(rental_end - lease_itr->rental_start <= MAX_LEASE_SECONDS, "rental_end exceeds the maximum lease duration"); name title_owner = lease_itr->title_owner; name renter = lease_itr->renter; - - assets_t renter_assets = get_assets(renter); - auto asset_itr = renter_assets.require_find(asset_id, "renter no longer owns the asset"); - name collection_name = asset_itr->collection_name; + name collection_name = lease_itr->collection_name; leases.modify(lease_itr, market, [&](auto &_lease) { _lease.rental_end = rental_end; }); - action( - permission_level{get_self(), name("active")}, - get_self(), - name("loglock"), - make_tuple(collection_name, asset_id, title_owner, renter, rental_end) - ).send(); + send_loglock(collection_name, asset_id, title_owner, renter, rental_end); } /** -* Permissionless reclaim of an expired lease: returns ownership from the renter -* to the title_owner and clears the lock. Callable by anyone once the lease has -* expired; the renter's signature is never required (the move runs under the -* contract's own authority). This is the guaranteed revert the whole model rests -* on. +* Permissionless reclaim of an expired lease: returns the asset to the title_owner and clears the +* lock, under the contract's own authority (no renter signature). The guaranteed revert the model +* rests on. * @required_auth none (permissionless) */ ACTION atomicassets::reclaim( @@ -236,21 +211,11 @@ ACTION atomicassets::reclaim( name title_owner = lease_itr->title_owner; name renter = lease_itr->renter; + name collection_name = lease_itr->collection_name; - assets_t renter_assets = get_assets(renter); - auto asset_itr = renter_assets.require_find(asset_id, "renter no longer owns the asset"); - name collection_name = asset_itr->collection_name; - - // Erase the lock, then move the asset back under the contract's own authority - // (enforce_lock=false). The contract pays any transient scope RAM so reclaim - // never needs the title_owner's or renter's signature. The move emits the normal - // logtransfer (notifying the asset's collection), and logreclaim below notifies - // the collection of the structured reclaim event. Crucially, NEITHER notifies the - // renter or title_owner: a renter is an arbitrary (possibly hostile) account, and - // notifying it would let it abort this guaranteed revert by throwing in a handler - // and trap the asset forever. A pre-existing offer referencing the asset is left - // in place (it could not settle while locked, and becomes valid again now that the - // asset is back with its owner). + // Erase the lock, then move the asset back under the contract's own authority (enforce_lock=false; + // contract pays transient scope RAM). See logreclaim for why this path notifies no account that + // could abort it. leases.erase(lease_itr); internal_transfer(renter, title_owner, vector{asset_id}, "lease reclaim", get_self(), false); @@ -298,9 +263,8 @@ ACTION atomicassets::createcol( check(allow_notify || notify_accounts.size() == 0, "Can't add notify_accounts if allow_notify is false"); - // createcol writes both vectors verbatim; cap them at 24 like addcolauth/addnotifyacc, and - // before the loops below so an oversized vector fails fast. The cap keeps partial_read_collection - // within its read budget. + // Cap both vectors at 24 (like addcolauth/addnotifyacc), before the loops, so an oversized + // vector fails fast and stays within partial_read_collection's read budget. check(authorized_accounts.size() <= 24, "Can only have up to 24 authorized accounts"); check(notify_accounts.size() <= 24, "Can only have up to 24 notify accounts"); @@ -1661,17 +1625,11 @@ ACTION atomicassets::logreclaim( ) { require_auth(get_self()); - // The asset's collection is notified of the reclaim (mirrors loglock on lease-start), so a - // collection can react to its assets returning. This trusts collections not to grief their - // own collection: a collection notify-account that throws here CAN abort the reclaim and trap - // the asset, which is accepted under the same trust model that lets collections gate transfers. - // - // Deliberately NO require_recipient(renter) / require_recipient(title_owner): reclaim is the - // permissionless guaranteed revert the whole model rests on. The renter is an arbitrary, - // possibly hostile account that profits from keeping the asset; notifying it would hand it a - // veto (throw in a handler -> abort the reclaim -> asset trapped forever). The title_owner is - // the beneficiary and learns of the reclaim by receiving the asset and this trace, so there is - // no reason to give it an abort lever either. + // Notify the collection (mirrors loglock) so it can react to its assets returning. This trusts + // collections not to grief their own: a throwing notify-account CAN abort the reclaim - accepted, + // same as collections gating transfers. But NOT the renter or title_owner: the renter is an + // arbitrary account that profits from keeping the asset, so notifying it would let it veto the + // guaranteed revert (throw in the handler -> reclaim aborts -> asset trapped). notify_collection_accounts(collection_name); } @@ -1943,12 +1901,9 @@ void atomicassets::internal_transfer( //to assets are empty => no scope has been created yet bool no_previous_scope = to_assets.begin() == to_assets.end(); if (no_previous_scope) { - //A dummy asset is emplaced, which makes the scope_payer pay for the ram of the scope - //This asset is later deleted again. - //This requires scope_payer to have authorized the action - EXCEPT when scope_payer is - //the contract itself (get_self()), which can always bill its own RAM. The privileged - //rental paths (leasestart, permissionless reclaim) rely on that: they pass get_self() - //so no title_owner/renter signature is needed to create the destination scope. + //A dummy asset is emplaced so scope_payer pays the new scope's RAM; it is deleted below. + //scope_payer must have authorized the action - except get_self(), which always bills its + //own RAM (the rental paths pass get_self(), so no renter/title_owner signature is needed). to_assets.emplace(scope_payer, [&](auto &_asset) { _asset.asset_id = ULLONG_MAX; _asset.collection_name = name(""); @@ -2015,6 +1970,22 @@ name atomicassets::check_rental_market() { } +void atomicassets::send_loglock( + name collection_name, + uint64_t asset_id, + name title_owner, + name renter, + uint32_t rental_end +) { + action( + permission_level{get_self(), name("active")}, + get_self(), + name("loglock"), + make_tuple(collection_name, asset_id, title_owner, renter, rental_end) + ).send(); +} + + /** * Decreases the balance of a specified account by a specified quantity * If the specified account does not have at least as much tokens in the balance as should be removed diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index af005d9..3ccd120 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -54,6 +54,13 @@ describe("non-custodial rental primitives", () => { blockchain.resetTables(); await atomicassets.actions.init([]).send(`${atomicassets.name.toString()}@active`); + // Leasing is opt-in (rentalcfg defaults to name("") = disabled). Enable it for + // the rental tests by authorizing the market; the "default disabled" test below + // covers the un-configured state explicitly. + await atomicassets.actions.setrentmkt([ + market.name.toString() + ]).send(`${atomicassets.name.toString()}@active`); + await atomicassets.actions.createcol([ lister.name.toString(), "testcollect1", @@ -141,13 +148,27 @@ describe("non-custodial rental primitives", () => { asset_id: ASSET1, title_owner: lister.name.toString(), renter: renter.name.toString(), + collection_name: "testcollect1", rental_start: rentalEnd - ONE_HOUR, rental_end: rentalEnd }]); }); - test("leasing works out of the box on the default rentalcfg (no setrentmkt needed)", async () => { + test("leasing is DISABLED by default; enabling requires setrentmkt", async () => { await mint(); + // reset rentalcfg to its on-deploy default (the unconfigured/disabled state) + await atomicassets.actions.setrentmkt([""]).send(`${atomicassets.name.toString()}@active`); + + const rentalEnd = nowSec() + ONE_HOUR; + await expect(atomicassets.actions.leasestart([ + lister.name.toString(), renter.name.toString(), + ASSET1, rentalEnd, "lease" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("Leasing is disabled"); + + // re-enable and confirm leasing works + await atomicassets.actions.setrentmkt([ + market.name.toString() + ]).send(`${atomicassets.name.toString()}@active`); await expect(leaseFor()).resolves.toBeDefined(); expect(assetsOf(renter)).toHaveLength(1); }); From 188c9b34ed1af375a1e4c171fe6031350e20fbb2 Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Tue, 30 Jun 2026 16:57:03 -0400 Subject: [PATCH 10/11] docs(rentals): fix stale rentalcfg-default comments in tests (Copilot review) The test comments still said rentalcfg defaults to "atomicmarket"; it now defaults to disabled (opt-in), and the suite enables leasing via setrentmkt. --- tests/asset-actions/renting-invariants.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index 3ccd120..5c3c7f4 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -14,7 +14,7 @@ const { TimePoint } = require("@wharfkit/antelope"); describe("non-custodial rental primitives", () => { let blockchain; let atomicassets; - let market; // configured rental market (rentalcfg default = "atomicmarket") + let market; // the rental market we authorize via setrentmkt (leasing is opt-in) let lister; // title_owner / lessor let renter; // becomes the AA owner during the lease let third; // unrelated third party / random reclaim caller @@ -40,8 +40,8 @@ describe("non-custodial rental primitives", () => { beforeAll(async () => { blockchain = new Blockchain(); atomicassets = blockchain.createContract('atomicassets', './build/atomicassets'); - // The rentalcfg singleton defaults to "atomicmarket", so create that - // account as the authorized market (no setrentmkt needed). + // Leasing is opt-in (rentalcfg defaults to disabled); this is the market account we + // authorize via setrentmkt in beforeEach. market = blockchain.createAccount('atomicmarket'); lister = blockchain.createAccount('lister'); renter = blockchain.createAccount('renter'); From 21c998844978381430b8f2117ba0ba81f91a1dfc Mon Sep 17 00:00:00 2001 From: Rob Konsdorf Date: Thu, 2 Jul 2026 13:12:49 -0400 Subject: [PATCH 11/11] feat(rentals): rental_id linkage, richer lease logs, governance lease cap Greenfield hardening from the design review - table shapes and log ABIs are free to change now (no mainnet rows) and near-impossible later: - leases row + leasestart carry an opaque market-side rental_id, echoed in loglock and logreclaim, so indexers can join a reclaim to the rental that opened the lease structurally instead of parsing memo text - loglock now carries rental_start: consumers can compute duration and tell a lease start (rental_start == now) from an extension without a table read that races the trace - rentalcfg gains a governance-settable max_lease_seconds (setleasecap), bounded above by the compile-time MAX_LEASE_SECONDS protocol ceiling; the singleton's field set freezes at its first mainnet write, so it is decided now. setrentmkt and setleasecap preserve each other's fields --- include/atomicassets-interface.hpp | 1 + include/atomicassets.hpp | 23 +++++- src/atomicassets.cpp | 64 ++++++++++++--- .../asset-actions/renting-invariants.test.js | 80 ++++++++++++++++--- 4 files changed, 142 insertions(+), 26 deletions(-) diff --git a/include/atomicassets-interface.hpp b/include/atomicassets-interface.hpp index 4875888..24043c9 100644 --- a/include/atomicassets-interface.hpp +++ b/include/atomicassets-interface.hpp @@ -155,6 +155,7 @@ namespace atomicassets { name collection_name; uint32_t rental_start; uint32_t rental_end; + uint64_t rental_id; uint64_t primary_key() const { return asset_id; }; uint64_t by_title_owner() const { return title_owner.value; }; diff --git a/include/atomicassets.hpp b/include/atomicassets.hpp index c6f885d..10bc779 100644 --- a/include/atomicassets.hpp +++ b/include/atomicassets.hpp @@ -42,11 +42,16 @@ CONTRACT atomicassets : public contract { name rental_market ); + ACTION setleasecap( + uint32_t max_lease_seconds + ); + ACTION leasestart( name title_owner, name renter, uint64_t asset_id, uint32_t rental_end, + uint64_t rental_id, string memo ); @@ -282,14 +287,17 @@ CONTRACT atomicassets : public contract { uint64_t asset_id, name title_owner, name renter, - uint32_t rental_end + uint32_t rental_start, + uint32_t rental_end, + uint64_t rental_id ); ACTION logreclaim( name collection_name, uint64_t asset_id, name title_owner, - name renter + name renter, + uint64_t rental_id ); ACTION lognewoffer( @@ -472,6 +480,7 @@ CONTRACT atomicassets : public contract { name collection_name; uint32_t rental_start; // sec_since_epoch the lease was first opened (fixed across extensions) uint32_t rental_end; // sec_since_epoch the lease expires + uint64_t rental_id; // opaque market-side rental id, echoed in loglock/logreclaim uint64_t primary_key() const { return asset_id; }; uint64_t by_title_owner() const { return title_owner.value; }; @@ -528,7 +537,10 @@ CONTRACT atomicassets : public contract { // absent row) means disabled, so a fresh deploy is off until setrentmkt("atomicmarket"); set it // back to name("") to kill-switch all leasing. Not hardcoded - the market account differs per chain. TABLE rentalcfg_s { - name rental_market = name(""); + name rental_market = name(""); + // Governance-settable lease-duration cap, bounded above by the compile-time + // MAX_LEASE_SECONDS protocol ceiling (see setleasecap). + uint32_t max_lease_seconds = MAX_LEASE_SECONDS; }; typedef singleton rentalcfg_t; @@ -612,7 +624,10 @@ CONTRACT atomicassets : public contract { name check_rental_market(); // Emits the loglock action (shared by leasestart and leaseextend). - void send_loglock(name collection_name, uint64_t asset_id, name title_owner, name renter, uint32_t rental_end); + void send_loglock(name collection_name, uint64_t asset_id, name title_owner, name renter, + uint32_t rental_start, uint32_t rental_end, uint64_t rental_id); + + uint32_t get_lease_cap(); void notify_collection_accounts( name collection_name diff --git a/src/atomicassets.cpp b/src/atomicassets.cpp index ddc2d3a..3fdba78 100644 --- a/src/atomicassets.cpp +++ b/src/atomicassets.cpp @@ -97,7 +97,29 @@ ACTION atomicassets::setrentmkt(name rental_market) { check(rental_market == name("") || is_account(rental_market), "rental_market account does not exist"); - get_rentalcfg().set(rentalcfg_s{rental_market}, get_self()); + rentalcfg_t rentalcfg = get_rentalcfg(); + rentalcfg_s cfg = rentalcfg.get_or_default(rentalcfg_s{}); + cfg.rental_market = rental_market; + rentalcfg.set(cfg, get_self()); +} + + +/** +* Sets the lease-duration cap, bounded above by the MAX_LEASE_SECONDS protocol +* ceiling. Disabling leasing entirely is setrentmkt's job, so zero is rejected. +* @required_auth The contract itself +*/ +ACTION atomicassets::setleasecap(uint32_t max_lease_seconds) { + require_auth(get_self()); + + check(max_lease_seconds > 0, "max_lease_seconds must be positive"); + check(max_lease_seconds <= MAX_LEASE_SECONDS, + "max_lease_seconds exceeds the protocol ceiling"); + + rentalcfg_t rentalcfg = get_rentalcfg(); + rentalcfg_s cfg = rentalcfg.get_or_default(rentalcfg_s{}); + cfg.max_lease_seconds = max_lease_seconds; + rentalcfg.set(cfg, get_self()); } @@ -112,6 +134,7 @@ ACTION atomicassets::leasestart( name renter, uint64_t asset_id, uint32_t rental_end, + uint64_t rental_id, string memo ) { name market = check_rental_market(); @@ -123,7 +146,7 @@ ACTION atomicassets::leasestart( check(rental_end > now, "rental_end must be in the future"); // Protocol backstop against a compromised/buggy market minting a near-permanent lock. - check(rental_end - now <= MAX_LEASE_SECONDS, "rental_end exceeds the maximum lease duration"); + check(rental_end - now <= get_lease_cap(), "rental_end exceeds the maximum lease duration"); leases_t leases = get_leases(); check(leases.find(asset_id) == leases.end(), "Asset is already leased"); @@ -148,13 +171,14 @@ ACTION atomicassets::leasestart( _lease.collection_name = collection_name; _lease.rental_start = now; _lease.rental_end = rental_end; + _lease.rental_id = rental_id; }); // Flip lister -> renter under the contract's own authority: the lock is in force, so this is // the privileged enforce_lock=false path, and the contract pays any transient scope RAM. internal_transfer(title_owner, renter, vector{asset_id}, memo, get_self(), false); - send_loglock(collection_name, asset_id, title_owner, renter, rental_end); + send_loglock(collection_name, asset_id, title_owner, renter, now, rental_end, rental_id); } @@ -178,19 +202,23 @@ ACTION atomicassets::leaseextend( check(rental_end > lease_itr->rental_end, "rental_end must be later than the current end"); - // Cap the total window from the fixed rental_start, so repeated extensions can't roll past the max. - check(rental_end - lease_itr->rental_start <= MAX_LEASE_SECONDS, + // Cap the total window from the fixed rental_start, so repeated extensions can't roll past the + // max. Deliberate asymmetry: a reclaim + re-lease gets a fresh window, because it necessarily + // transits the reclaimable state the cap exists to guarantee. + check(rental_end - lease_itr->rental_start <= get_lease_cap(), "rental_end exceeds the maximum lease duration"); name title_owner = lease_itr->title_owner; name renter = lease_itr->renter; name collection_name = lease_itr->collection_name; + uint32_t rental_start = lease_itr->rental_start; + uint64_t rental_id = lease_itr->rental_id; leases.modify(lease_itr, market, [&](auto &_lease) { _lease.rental_end = rental_end; }); - send_loglock(collection_name, asset_id, title_owner, renter, rental_end); + send_loglock(collection_name, asset_id, title_owner, renter, rental_start, rental_end, rental_id); } @@ -212,6 +240,7 @@ ACTION atomicassets::reclaim( name title_owner = lease_itr->title_owner; name renter = lease_itr->renter; name collection_name = lease_itr->collection_name; + uint64_t rental_id = lease_itr->rental_id; // Erase the lock, then move the asset back under the contract's own authority (enforce_lock=false; // contract pays transient scope RAM). See logreclaim for why this path notifies no account that @@ -223,7 +252,7 @@ ACTION atomicassets::reclaim( permission_level{get_self(), name("active")}, get_self(), name("logreclaim"), - make_tuple(collection_name, asset_id, title_owner, renter) + make_tuple(collection_name, asset_id, title_owner, renter, rental_id) ).send(); } @@ -1608,7 +1637,9 @@ ACTION atomicassets::loglock( uint64_t asset_id, name title_owner, name renter, - uint32_t rental_end + uint32_t rental_start, + uint32_t rental_end, + uint64_t rental_id ) { require_auth(get_self()); @@ -1621,7 +1652,8 @@ ACTION atomicassets::logreclaim( name collection_name, uint64_t asset_id, name title_owner, - name renter + name renter, + uint64_t rental_id ) { require_auth(get_self()); @@ -1975,17 +2007,27 @@ void atomicassets::send_loglock( uint64_t asset_id, name title_owner, name renter, - uint32_t rental_end + uint32_t rental_start, + uint32_t rental_end, + uint64_t rental_id ) { action( permission_level{get_self(), name("active")}, get_self(), name("loglock"), - make_tuple(collection_name, asset_id, title_owner, renter, rental_end) + make_tuple(collection_name, asset_id, title_owner, renter, rental_start, rental_end, rental_id) ).send(); } +/** +* The governance-configured lease-duration cap (defaults to the MAX_LEASE_SECONDS ceiling). +*/ +uint32_t atomicassets::get_lease_cap() { + return get_rentalcfg().get_or_default(rentalcfg_s{}).max_lease_seconds; +} + + /** * Decreases the balance of a specified account by a specified quantity * If the specified account does not have at least as much tokens in the balance as should be removed diff --git a/tests/asset-actions/renting-invariants.test.js b/tests/asset-actions/renting-invariants.test.js index 5c3c7f4..094fc42 100644 --- a/tests/asset-actions/renting-invariants.test.js +++ b/tests/asset-actions/renting-invariants.test.js @@ -120,6 +120,9 @@ describe("non-custodial rental primitives", () => { return ASSET1; } + // The opaque market-side rental id threaded through leasestart/loglock/logreclaim. + const RENTAL_ID = 7; + // Opens a lease (market-signed) for the given duration. async function leaseFor(seconds = ONE_HOUR) { const rentalEnd = nowSec() + seconds; @@ -128,6 +131,7 @@ describe("non-custodial rental primitives", () => { renter.name.toString(), ASSET1, rentalEnd, + RENTAL_ID, "lease start" ]).send(`${market.name.toString()}@active`); return rentalEnd; @@ -150,7 +154,8 @@ describe("non-custodial rental primitives", () => { renter: renter.name.toString(), collection_name: "testcollect1", rental_start: rentalEnd - ONE_HOUR, - rental_end: rentalEnd + rental_end: rentalEnd, + rental_id: RENTAL_ID }]); }); @@ -162,7 +167,7 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "lease" + ASSET1, rentalEnd, RENTAL_ID, "lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("Leasing is disabled"); // re-enable and confirm leasing works @@ -179,7 +184,7 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "second lease" + ASSET1, rentalEnd, RENTAL_ID, "second lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("already leased"); }); @@ -188,13 +193,13 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "lease" + ASSET1, rentalEnd, RENTAL_ID, "lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("not transferable"); }); test("leaseextend bumps the end without changing ownership", async () => { await mint(); - await leaseFor(); + const rentalEnd = await leaseFor(); const newEnd = nowSec() + ONE_HOUR * 5; await atomicassets.actions.leaseextend([ ASSET1, newEnd @@ -202,6 +207,11 @@ describe("non-custodial rental primitives", () => { expect(assetsOf(renter)).toHaveLength(1); // still the renter's expect(leases()[0].rental_end).toBe(newEnd); + // rental_start and rental_id are FIXED across extensions: rental_start anchors + // the duration cap, and rental_id keeps identifying the lease-opening rental + // (extension payments carry their own ids in the market's logrental). + expect(leases()[0].rental_start).toBe(rentalEnd - ONE_HOUR); + expect(leases()[0].rental_id).toBe(RENTAL_ID); }); test("leaseextend cannot revive an expired lease (no racing the reclaim)", async () => { @@ -275,7 +285,7 @@ describe("non-custodial rental primitives", () => { // required authority of the configured market is missing await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "lease" + ASSET1, rentalEnd, RENTAL_ID, "lease" ]).send(`${third.name.toString()}@active`)).rejects.toThrow("missing required authority"); }); @@ -294,12 +304,12 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "lease" + ASSET1, rentalEnd, RENTAL_ID, "lease" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("missing required authority"); await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, rentalEnd, "lease" + ASSET1, rentalEnd, RENTAL_ID, "lease" ]).send(`${third.name.toString()}@active`)).resolves.not.toThrow(); expect(assetsOf(renter)).toHaveLength(1); }); @@ -383,7 +393,7 @@ describe("non-custodial rental primitives", () => { const rentalEnd = nowSec() + ONE_HOUR; await atomicassets.actions.leasestart([ lister.name.toString(), evil.name.toString(), - ASSET1, rentalEnd, "lease to evil renter" + ASSET1, rentalEnd, RENTAL_ID, "lease to evil renter" ]).send(`${market.name.toString()}@active`); expect(assetsOf(evil)).toHaveLength(1); @@ -428,7 +438,7 @@ describe("non-custodial rental primitives", () => { const tooLong = nowSec() + MAX_LEASE_SECONDS + ONE_HOUR; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, tooLong, "too long" + ASSET1, tooLong, RENTAL_ID, "too long" ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); }); @@ -437,7 +447,7 @@ describe("non-custodial rental primitives", () => { const atCap = nowSec() + MAX_LEASE_SECONDS; await expect(atomicassets.actions.leasestart([ lister.name.toString(), renter.name.toString(), - ASSET1, atCap, "at cap" + ASSET1, atCap, RENTAL_ID, "at cap" ]).send(`${market.name.toString()}@active`)).resolves.not.toThrow(); expect(assetsOf(renter)).toHaveLength(1); }); @@ -451,4 +461,52 @@ describe("non-custodial rental primitives", () => { ASSET1, rentalStart + MAX_LEASE_SECONDS + ONE_HOUR ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); }); + + // ----------------------------------- governance cap (setleasecap, <= protocol ceiling) + + test("setleasecap lowers the cap for leasestart and leaseextend, bounded by the ceiling", async () => { + await mint(); + + // only the contract may set it, and never above the compile-time ceiling or to zero + await expect(atomicassets.actions.setleasecap([ + ONE_HOUR + ]).send(`${lister.name.toString()}@active`)).rejects.toThrow("missing required authority"); + await expect(atomicassets.actions.setleasecap([ + MAX_LEASE_SECONDS + 1 + ]).send(`${atomicassets.name.toString()}@active`)).rejects.toThrow("exceeds the protocol ceiling"); + await expect(atomicassets.actions.setleasecap([ + 0 + ]).send(`${atomicassets.name.toString()}@active`)).rejects.toThrow("must be positive"); + + // cap to 2 hours: a 3-hour lease is rejected, a 2-hour lease passes + await atomicassets.actions.setleasecap([ + 2 * ONE_HOUR + ]).send(`${atomicassets.name.toString()}@active`); + await expect(atomicassets.actions.leasestart([ + lister.name.toString(), renter.name.toString(), + ASSET1, nowSec() + 3 * ONE_HOUR, RENTAL_ID, "too long for cap" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); + const rentalEnd = await leaseFor(2 * ONE_HOUR); + + // the lowered cap also bounds the total extended window from rental_start + await expect(atomicassets.actions.leaseextend([ + ASSET1, rentalEnd + ONE_HOUR + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); + }); + + test("setrentmkt preserves the configured cap (and vice versa)", async () => { + await mint(); + await atomicassets.actions.setleasecap([ + 2 * ONE_HOUR + ]).send(`${atomicassets.name.toString()}@active`); + + // re-pointing the market must not reset max_lease_seconds to the default + await atomicassets.actions.setrentmkt([ + market.name.toString() + ]).send(`${atomicassets.name.toString()}@active`); + await expect(atomicassets.actions.leasestart([ + lister.name.toString(), renter.name.toString(), + ASSET1, nowSec() + 3 * ONE_HOUR, RENTAL_ID, "over the preserved cap" + ]).send(`${market.name.toString()}@active`)).rejects.toThrow("maximum lease duration"); + }); });