Experimental: non-custodial rental flow over AA lease primitives - #11
Draft
robrigo wants to merge 15 commits into
Draft
Experimental: non-custodial rental flow over AA lease primitives#11robrigo wants to merge 15 commits into
robrigo wants to merge 15 commits into
Conversation
…itives Rewrite the rental flow to be non-custodial: the asset is never escrowed. announcerent only records the listing; the asset stays with the lister until rented. rentasset drives the AtomicAssets leasestart (making the renter the real owner and locking the asset), leaseextend for same-renter extensions, and reclaim-then-leasestart for an expired-but-unreclaimed re-rental. endrent and cancelrent become thin reclaim + mirror-resync wrappers. - remove the "rental" custody intake from receive_asset_transfer and the now-unused logrentstart - rename rentals.asset_transferred to is_rented - refuse listing a rental-locked asset for sale/auction/buyoffer/re-rent via a single check in get_collection_and_check_assets (reads the AA leases table directly); AA's own guards remain the authoritative backstop - read the AA leases table as the source of truth for lock state, and pass the asset's current owner as the royalty payout scope - update the AA test fixture to the non-custodial build and rewrite the rental lifecycle suite + add lock-refusal and stray-memo tests Experimental branch for smart-contract review. Pairs with the atomicassets-contract experiment/noncustodial-rentals branch.
…sh fixture) - A leases row always implies an active lease now, so simplify the lease_exists checks (drop the redundant renter!="" clause) in rentasset/endrent/cancelrent. - Refresh the bundled AtomicAssets test fixture to the updated non-custodial build (pretitle/delpretitle removed, rentalcfg singleton). Sale/rental cross-listing is intentionally left to coexist: the losing listing reverts safely and is swept by atomicmarket-cronjobs (a cancel-invalid-rentals follow-on, mirroring cancel-invalid-sales).
There was a problem hiding this comment.
Pull request overview
This PR reworks AtomicMarket’s rental feature from a custodial escrow model to a non-custodial model driven by AtomicAssets “lease” primitives, where the renter becomes the on-chain AtomicAssets owner for the lease term and the lock state is sourced from the AA leases table.
Changes:
- Replaces the custodial rental activation/holdership flow with AA
leasestart/leaseextend/ permissionlessreclaim, and treats AAleasesas the lock source of truth. - Removes the
"rental"memo intake path (and related logging), and adds early lock-refusal when listing assets. - Updates the AtomicAssets test fixture ABI and rewrites/extends smoke tests to assert the new ownership + leases-table behavior.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/atomicmarket.cpp |
Implements non-custodial rental lifecycle (announce/rent/extend/end/cancel) and lock-refusal via AA leases. |
include/atomicmarket.hpp |
Updates rental flow documentation and renames the rental row flag to reflect non-custodial semantics. |
include/atomicassets-interface.hpp |
Replaces the legacy holders interface with the new leases table interface. |
tests/fixtures/atomicassets/atomicassets.abi |
Updates the bundled AtomicAssets ABI to include lease primitives/table and remove legacy custody-related pieces. |
tests/market-smoke.test.js |
Updates integration tests for the new non-custodial rental behavior and adds lock-refusal + stray-memo rejection coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1921
to
+1925
| } else { | ||
| // Not rented: the asset is still with the lister, nothing to return. Anyone may cancel an | ||
| // invalid listing (the owner no longer owns the asset); otherwise the owner must authorize. | ||
| atomicassets::assets_t owner_assets = atomicassets::get_assets(rental_itr->owner); | ||
| bool is_rental_invalid = owner_assets.find(asset_id) == owner_assets.end(); |
Contributor
Author
There was a problem hiding this comment.
Intentional for this experiment: the non-custodial model never escrows the asset, and it deploys with no legacy custodial rentals in flight (nothing post-flight), so a missing lease row always means the lister still holds the asset. Documented that precondition in cb16f50. If this ever lands on a chain with active custodial rentals, those would be drained first as a separate migration step (tracked as a downstream follow-on).
Comment on lines
+151
to
+153
| // Non-custodial rental lock/title record. A row's existence means the asset | ||
| // is leased and locked; renter == name("") && rental_end == 0 is a pretitle | ||
| // sentinel (locked, owner still the lister). |
Contributor
Author
There was a problem hiding this comment.
Fixed in cb16f50: dropped the stale pretitle-sentinel description; a leases row now always means an active lease.
Pick up the updated non-custodial atomicassets build so the integration tests run against the leaseextend-after-expiry guard added in atomicassets-contract.
…dition (Copilot review) - leases_s interface comment and the lock-refusal comment no longer mention the removed "pretitle" sentinel; a leases row always means an active lease. - Document that cancelrent's non-rented branch assumes the non-custodial deploy has no legacy custodial (escrowed) rentals in flight, so "owner no longer owns it" always means the lister moved the asset, never contract-escrowed state.
Comment on lines
+2012
to
+2017
| // The royalty engine reads the asset's data from its CURRENT owner scope. Non-custodially | ||
| // that is whoever owns the asset right now (the inline lease actions below run only after | ||
| // this action returns): the current renter for an extension or an expired-unreclaimed | ||
| // re-rental, otherwise the listing owner. | ||
| name asset_scope = lease_exists ? lease_itr->renter : rental_itr->owner; | ||
|
|
Contributor
Author
There was a problem hiding this comment.
No longer applicable: the pretitle/sentinel state was removed in this PR. A leases row always has a non-empty renter (the renter is the real owner), so asset_scope is always a real account, never name("").
Comment on lines
+1910
to
+1914
| if (lease_exists) { | ||
| // Expired but never reclaimed: only the owner can cancel, and the asset returns to them | ||
| // via the permissionless reclaim. | ||
| require_auth(rental_itr->owner); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
No longer applicable: pretitle/sentinels were removed. A leases row always represents a real (active or expired) lease, and cancelrent only fires reclaim when active_lease is false (expired-but-unreclaimed); there is no sentinel case to mishandle.
Comment on lines
679
to
688
| TABLE rentals_s { | ||
| uint64_t asset_id; | ||
| name owner; // the listing creator; receives the rental payouts | ||
| name holder; // the current renter; name("") when not rented out | ||
| asset price_per_hour; // denoted in the listing symbol | ||
| symbol settlement_symbol; // what the rental is actually paid in | ||
| uint32_t maximum_rental_duration; // seconds; the longest period a rental can cover | ||
| uint32_t rental_end; // seconds since epoch; 0 when not rented out | ||
| bool asset_transferred; // true once the asset is in contract custody | ||
| bool is_rented; // true while a renter holds an active lease | ||
| name maker_marketplace; |
Comment on lines
31
to
35
| const aaTables = { | ||
| assets: (scope) => atomicassets.tables.assets(nameToBigInt(Name.from(scope))).getTableRows(), | ||
| holders: () => atomicassets.tables.holders(nameToBigInt(atomicassets.name)).getTableRows(), | ||
| leases: () => atomicassets.tables.leases(nameToBigInt(atomicassets.name)).getTableRows(), | ||
| offers: () => atomicassets.tables.offers(nameToBigInt(atomicassets.name)).getTableRows(), | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
Already updated (commit 78f42f1): the suite header now describes the non-custodial leases flow, not the custodial holders model.
…a rolling window The maximum_rental_duration check measured the remaining period from "now", so a renter who kept paying could perpetually extend within a rolling window and hold the asset indefinitely, leaving the title owner no guaranteed return. Cap the total from the lease's rental_start (read from the AtomicAssets leases table) instead. Refresh the AA fixture; add a regression test that a from-now-valid but from-start-excessive extension is rejected.
…ield
The rentals row carried two redundant fields for one fact: holder (write-only,
never read) and is_rented (read once in endrent), while the AtomicAssets leases
table is the real source of truth. Collapse to a single `renter` name field
(name("") when not rented, the renter otherwise) that doubles as the rented
flag and carries the renter identity for indexers. Rename holder -> renter to
match the AtomicAssets leases terminology. Refresh the AA fixture and tests.
…line calls Match the AtomicAssets simplification: the leases interface has no market column and leasestart/leaseextend take no market argument. Update the inline rentasset calls, the fixture, and the test assertions.
Comment on lines
+2117
to
+2130
| // If the lease still exists in AtomicAssets it must have expired; trigger the permissionless | ||
| // reclaim. If a keeper already reclaimed it, the row is gone and we just resync the mirror. | ||
| atomicassets::leases_t aa_leases = atomicassets::get_leases(); | ||
| auto lease_itr = aa_leases.find(asset_id); | ||
| if (lease_itr != aa_leases.end()) { | ||
| check(lease_itr->rental_end <= current_time, "The rental period is not over yet"); | ||
|
|
||
| action( | ||
| permission_level{get_self(), name("active")}, | ||
| atomicassets::ATOMICASSETS_ACCOUNT, | ||
| name("reclaim"), | ||
| make_tuple(asset_id) | ||
| ).send(); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
On-chain reads are synchronous and consistent, so AA state is never 'temporarily unavailable' here. A missing leases row means the lease was reclaimed, and reclaim requires now >= rental_end, so it was already expired. When the row still exists, the active branch enforces check(rental_end <= now). So endrent can never reset the mirror on a still-active lease.
Comment on lines
+2543
to
+2545
| // A rental-locked (leased) asset cannot be listed for sale, auction or | ||
| // buyoffer. Refuse it early here rather than letting it fail at settlement time. The | ||
| // AtomicAssets transfer/offer guards remain the authoritative backstop. |
Contributor
Author
There was a problem hiding this comment.
Fixed in e8613b7 - the comment now states the helper guards every caller, including announcerent (a new rental listing), not just sale/auction/buyoffer.
Comment on lines
+323
to
+332
| 2. A renter pays for a number of hours from their deposited balance (rentasset). The | ||
| payment is distributed like a sale payout (market fees, collection fee / royalty | ||
| splits, remainder to the listing owner) and the atomicassets HOLDERSHIP of the | ||
| asset is moved to the renter, while ownership stays with the contract | ||
| 4. After the rental period is over, anyone can reset the holdership back to the | ||
| contract (endrent), making the listing rentable again | ||
| 5. The owner can cancel the listing and reclaim the asset whenever no rental is | ||
| actively running (cancelrent) | ||
| splits, remainder to the listing owner) and atomicmarket drives AtomicAssets to make | ||
| the RENTER the real owner of the asset (leasestart), parking the lister's reclaim | ||
| right in the AtomicAssets leases table. The asset is locked while leased. | ||
| 3. An extension by the same renter only bumps the lease end (leaseextend); no second | ||
| ownership flip. | ||
| 4. After the rental period is over, the asset returns to the lister via the permissionless | ||
| AtomicAssets `reclaim` (a keeper, endrent, or cancelrent triggers it). atomicmarket's | ||
| endrent then resyncs its mirror row, making the listing rentable again. |
Contributor
Author
There was a problem hiding this comment.
Fixed the PR description to match the implemented seam: leasestart(title_owner, renter, asset_id, rental_end, memo) and leaseextend(asset_id, rental_end), no market arg. AtomicAssets authorizes via require_auth(rentalcfg.rental_market) (set with setrentmkt).
The integration suite runs against a snapshot of the non-custodial AA contract. Update it so the tests exercise the un-vetoable reclaim and the lease-duration backstop (atomicassets-contract#26). No interface change; AM 231 pass.
Contributor
Author
Security review — orchestration side (paired with atomicassets/atomicassets-contract#26)Reviewed alongside the AA primitives. The Critical (renter-vetoable reclaim) and High (no protocol duration cap) issues were in AtomicAssets and are fixed there (atomicassets/atomicassets-contract#26). The orchestration here checks out:
🟡 Medium (your call) — sale + rental coexistence
The bundled AtomicAssets test fixture was refreshed to the reclaim-hardened build so the integration suite exercises the fixed primitives (231 pass). |
…ce of truth Post-review cleanup of the non-custodial rental orchestration (pairs with atomicassets-contract#26): - The rentals table no longer mirrors lock state. renter/rental_end and the never-read rentalends index are removed; rentals_s is now immutable listing config and the AtomicAssets leases table is the single source of truth for who is renting and until when. endrent reads the lease, not a mirror row. - Pin the asset_scope royalty invariant with a comment and a regression test (an extension must pay template/attribute royalties from the renter scope). - Extract send_aa_reclaim to drop the triplicated reclaim emit. - Add collection_name to the leases interface struct (matches AA); enable leasing in the test setup and refresh the bundled AA fixture.
Comment on lines
2091
to
2097
| // AtomicAssets leases is the source of truth: a row means it's still rented; no row means a | ||
| // keeper already reclaimed it (nothing to do). | ||
| atomicassets::leases_t aa_leases = atomicassets::get_leases(); | ||
| auto lease_itr = aa_leases.find(asset_id); | ||
| check(lease_itr != aa_leases.end(), "This asset is not currently rented out"); | ||
| check(lease_itr->rental_end <= current_time_point().sec_since_epoch(), | ||
| "The rental period is not over yet"); |
Comment on lines
+330
to
+333
| 4. After the rental period is over, the asset returns to the lister via the permissionless | ||
| AtomicAssets `reclaim` (a keeper, endrent, or cancelrent triggers it). atomicmarket's | ||
| endrent then resyncs its mirror row, making the listing rentable again. | ||
| 5. The owner can cancel the listing whenever no rental is actively running (cancelrent); |
…review) - endrent now no-ops when the AtomicAssets lease row is already gone (a keeper beat it) instead of hard-failing, matching its wrapper intent and the comment. - The rental-flow doc no longer claims endrent "resyncs its mirror row" - there is no mirror; the AtomicAssets leases table is the single source of truth.
Comment on lines
+723
to
+732
| const leased = aaTables.leases(); | ||
| expect(leased).toEqual([{ | ||
| asset_id: ASSET1, | ||
| title_owner: 'seller', | ||
| renter: 'renter', | ||
| collection_name: COL, | ||
| rental_start: Number(leased[0].rental_end) - 2 * 3600, | ||
| rental_end: Number(leased[0].rental_end), | ||
| rental_id: 1, // the AM rental counter, echoed on the lease for indexer joins | ||
| }]); |
Comment on lines
+801
to
+810
| const leased = aaTables.leases(); | ||
| expect(leased).toEqual([{ | ||
| asset_id: ASSET1, | ||
| title_owner: 'seller', | ||
| renter: 'renter2', | ||
| collection_name: COL, | ||
| rental_start: Number(leased[0].rental_end) - 1 * 3600, | ||
| rental_end: Number(leased[0].rental_end), | ||
| rental_id: 2, // the re-lease is a NEW rental: fresh counter id on the fresh lease | ||
| }]); |
Comment on lines
+2036
to
+2037
| // The AtomicAssets leases table is the source of truth for the on-chain lock state; the | ||
| // rentals mirror below can lag a keeper-driven reclaim. |
Comment on lines
+330
to
+335
| 4. After the rental period is over, the asset returns to the lister via the permissionless | ||
| AtomicAssets `reclaim` (a keeper, endrent, or cancelrent triggers it). The listing is | ||
| rentable again as soon as the lease row is gone; atomicmarket keeps no lock state of its | ||
| own (the AtomicAssets leases table is the single source of truth). | ||
| 5. The owner can cancel the listing whenever no rental is actively running (cancelrent); | ||
| an expired-but-unreclaimed lease is reclaimed as part of the cancel. |
Comment on lines
+151
to
+153
| // Non-custodial rental lock/title record. A row's existence means the asset | ||
| // is actively leased and locked: the renter is the real owner and title_owner | ||
| // holds the reclaim right until rental_end. |
Comment on lines
2343
to
2347
| } else { | ||
| // Non-custodial rentals never transfer the asset to the contract, so a "rental" memo is | ||
| // no longer a valid intake and falls through to this rejection. | ||
| check(false, "Invalid memo"); | ||
| } |
robrigo
force-pushed
the
experiment/noncustodial-rentals
branch
from
July 2, 2026 17:27
acccaa8 to
92465d0
Compare
…al_id linkage Design-review follow-ups. The one economic fix: listing terms were frozen for the whole lease (cancelrent rejected during an active lease, announce requires not-leased), so at expiry anyone could atomically snipe a fresh full window at a stale price. The listing row is the owner's OFFER of future rentals, separate from the renter's purchased lease: - editrent reprices / re-bounds / re-attributes a listing at any time; the listing and settlement symbols stay immutable (expected_price pins amount + listing symbol only). Extensions pay the new terms; renters are protected by the existing expected_price_per_hour pin - cancelrent during an active lease is now an owner-auth delist that leaves the lease untouched; endrent is purely lease-driven and no longer needs a listing row - logrental carries rental_start and is_extension (extension windows were not derivable from the log), and its off-canon rental_counter_id param is renamed rental_id to match sale_id/auction_id/buyoffer_id - rentasset threads rental_id into AA leasestart, where it is stored on the lease and echoed in loglock/logreclaim for structural indexer joins - AA fixture refreshed to the rental_id-aware build
robrigo
force-pushed
the
experiment/noncustodial-rentals
branch
from
July 2, 2026 17:32
92465d0 to
a07f751
Compare
…hi extension Regression tests for three paths the security review flagged as unguarded: - endrent on an already-reclaimed asset is a no-op, not a throw - an invalid listing (owner moved the asset) is cancellable by anyone, while a still-valid one rejects a non-owner cancel - a delphi-priced rental extension settles the added hours at the oracle rate via the leaseextend path
The page still described the old custodial holder model, which the contract now rejects. Rewrite to renter-as-owner: leases is the source of truth, no rental-memo custody step, permissionless un-vetoable reclaim, idempotent endrent, and the current immutable rentals table shape.
robrigo
added a commit
that referenced
this pull request
Jul 3, 2026
Rentals are descoped from the V2 release so the rest of V2 can ship without them. Removed in full: - actions announcerent / cancelrent / rentasset / endrent / payrentram - log actions lognewrent / logrentstart / logrental - rentals table (+ rentals_s, rentalends index, get_rentals accessor) - the "rental" memo branch in receive_asset_transfer - holders_s / get_holders from the AtomicAssets interface header (the custodial holdership mechanism is being removed from AtomicAssets V2 in the companion PR atomicassets-contract#27) - ricardian clauses, wiki page, README/CLAUDE.md/api-integration.md sections internal_payout_sale, consume_counter and the counters table are shared with sales/auctions and stay; only the rental call sites are gone. The migrate action never seeded a rental counter, so no migration change. ABI diff vs v2.0.0-rc1: exactly the 8 rental actions + rentals table (and their structs) removed. Rentals live on: the custodial implementation is preserved on archive/v2-custodial-rentals + the v2.0.0-rc1 tag; the non-custodial rework continues on experiment/noncustodial-rentals (#11, #12). Tests: 8 rental cases + the rentals table helper removed from market-smoke; auctions.test.js kept (renter accounts there are generic bidders). Suite: 6 suites, 220 passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Experimental branch for smart-contract review. Pairs with atomicassets/atomicassets-contract#26 (the lock and title primitives this orchestrates); review the two together.
Summary
Switches the rental flow from custodial (asset escrowed in
atomicmarket) to non-custodial. The asset stays with the lister until it is rented, then the renter becomes the real AtomicAssets owner for the lease term. See atomicassets-contract#26 for the model rationale.Design
announcerentno longer escrows the asset; it stays owned and usable by the lister until rented.editrentchanges a listing's price, maximum duration and maker marketplace at any time, including while a lease runs: the listing row is the owner's offer of future rentals, separate from the renter's purchased lease. Without it, terms were frozen for the whole lease and the expiry second could be sniped at a stale price for a fresh full window. The listing and settlement symbols are immutable (expected_price_per_hourpins amount + listing symbol only, so a mutable settlement symbol could charge a multi-token depositor in an unintended token) — cancel and relist to change symbols. An extension by the incumbent renter is a fresh purchase of the current offer and pays the new terms; renters are protected by theexpected_price_per_hourpin.rentassetpays out like a sale, then drives the AtomicAssets primitives:leasestartfor a new rental (threading therental_idthat AA stores and echoes in its lease logs),leaseextendfor a same-renter extension, andreclaimthenleasestartfor an expired-but-unreclaimed re-rental. It reads the AAleasestable for lock state.cancelrentdelists at any time; delisting is not termination. During an active lease it is an owner-auth withdrawal of the offer only — the lease runs to its end and is reclaimed as usual. An expired-but-unreclaimed lease is reclaimed as part of the cancel; an invalid listing (owner no longer owns the unleased asset) may be cancelled by anyone.endrentis a thin wrapper over the permissionless AAreclaim: idempotent, and purely lease-driven (no listing row required), so keepers keep working for leases whose listing was delisted while they ran.get_collection_and_check_assetsreads the AAleasestable to refuse listing a rental-locked asset for sale, auction, buyoffer, or re-rent, failing early at listing time. AtomicAssets' own transfer and offer guards remain the authoritative backstop.State model
The
rentalstable holds the owner's offered terms (mutable viaeditrent): owner, price, settlement symbol, maximum duration, marketplace, collection, and fee. It does not mirror lock state. The AtomicAssetsleasestable is the single source of truth for who is renting and until when, and the two map 1:1 onasset_id.Royalty scope
rentassetruns the payout before the inline lease actions, so the asset is still owned by its current owner during the payout.asset_scopeis the lister for a fresh rental and the renter for an extension or expired re-rental, rather thanget_self()as in the custodial code. A regression test pins this so a future reorder fails loudly instead of silently misrouting royalties.Logs
lognewrent(listing created),logeditrent(terms changed),logrental(rental_id, asset, lister, renter, hours, paid settlement price,rental_start,rental_end,is_extension, taker marketplace).rental_start/is_extensionmake extension windows derivable from the log alone;rental_idreplaces the off-canonrental_counter_idname, matchingsale_id/auction_id/buyoffer_id. The AA-sideloglock/logreclaimecho the samerental_id, giving indexers a structural join from lease events to market rentals.AtomicAssets interface used
leasestart(title_owner, renter, asset_id, rental_end, rental_id, memo),leaseextend(asset_id, rental_end), and permissionlessreclaim(asset_id). AtomicAssets authorizes lease-start and lease-extend via itsrentalcfgsingleton; AtomicMarket never ends a lease itself.Tests
VeRT: 6 suites / 238 pass. The bundled AtomicAssets fixture is the rental_id-aware build, so the integration tests exercise the real primitives.
docs/wiki/Rentals.mdis rewritten for the non-custodial model (the old page described the custodial flow this branch removes). Covers the rental lifecycle,editrent(mid-lease repricing, the stale-price snipe regression, validation), delist-during-lease with the lease-drivenendrent, the lock-refusal on listing, stray-memo rejection, and the royalty-scope invariant.Follow-on (not in this PR)
cancel-invalid-salescron inatomicmarket-cronjobsthen sweeps.endrent/reclaim, an expired lease leaves the renter with the asset's utility for free; the platform keeper cron (tracked separately) sweeps expiries via the AArentalendindex. Re-rentals self-heal inline regardless.