feat!: remove custodial rentals from V2 - #13
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the custodial rentals feature set from AtomicMarket V2 (actions, tables, logs, and the "rental" transfer-memo branch), aligning the contract and docs with the decision to ship the remaining V2 features sooner while the AtomicAssets custody primitives are being removed upstream.
Changes:
- Deleted custodial rental actions/logs/state (
rentalstable + indices) and removed"rental"handling inreceive_asset_transfer. - Removed the AtomicAssets custodial
holdersinterface frominclude/atomicassets-interface.hppand updated contract/docs references accordingly. - Updated tests and docs to drop rental coverage/content; additionally lands the VeRT test-suite migration work from the integration base branch.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sales.test.js | Adds comprehensive VeRT coverage for v2 sale flow actions and receive_asset_offer. |
| tests/market-smoke.test.js | Removes rental-related smoke tests and table helpers; updates suite description accordingly. |
| tests/deposit-withdraw.test.js | Adds VeRT coverage for deposits/withdrawals and token-support/guard paths. |
| tests/buyoffers.test.js | Adds VeRT coverage for buyoffer lifecycle actions and validations. |
| tests/auctions.test.js | Adds VeRT coverage for auction lifecycle actions and validations. |
| tests/admin.test.js | Adds VeRT coverage for admin/config actions and validations. |
| src/atomicmarket.cpp | Removes rental action implementations, rental memo path, and rental log actions; updates related comments. |
| resource/atomicmarket.contracts.md | Removes Ricardian/spec docs for rental actions/logs and updates royalty-split wording. |
| README.md | Removes rentals from the project description and feature list. |
| include/atomicmarket.hpp | Removes rental action declarations, rental logs, rentals table definition, and get_rentals() accessor. |
| include/atomicassets-interface.hpp | Removes holders table struct and get_holders() accessor. |
| docs/wiki/V2-Changes.md | Removes rentals references from V2 change summary and indexer notes. |
| docs/wiki/Royalty-Splits.md | Updates wording to remove rentals references. |
| docs/wiki/Rentals.md | Deletes the rentals wiki page. |
| docs/wiki/README.md | Updates wiki-link example to no longer reference Rentals. |
| docs/wiki/Home.md | Removes rentals from feature list and adjusts conceptual descriptions to sales/auctions only. |
| docs/wiki/_Sidebar.md | Removes Rentals from the wiki sidebar. |
| docs/api-integration.md | Removes rentals workflow/table/action/memo documentation and renumbers sections accordingly. |
| CLAUDE.md | Removes rentals from repo overview and test-suite descriptions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
487
to
488
| * The founders category is a list of global recipients that applies to every sale of | ||
| * the collection. The three split weights determine how the collection fee is divided between |
Comment on lines
644
to
645
| * A rule matches when an asset that is sold has an attribute with the exact | ||
| * (source, field, value) triple of the rule. The value's type is part of the match - |
Comment on lines
2593
to
2597
| // Collection fee - the fee at EXECUTION time ALWAYS applies, regardless of the fee that | ||
| // was stored when the listing was created. This gives the collection author full control: | ||
| // both fee reductions and fee raises take effect immediately on every already-created | ||
| // listing (sales, auctions and rentals alike). The stored collection_fee is retained only | ||
| // listing (sales and auctions alike). The stored collection_fee is retained only | ||
| // for informational / indexing purposes (emitted by the lognew* actions) and no longer |
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.
robrigo
force-pushed
the
feat/remove-rentals
branch
from
July 3, 2026 00:17
bfe1bc6 to
3394669
Compare
Comment on lines
25
to
+27
| **Behavior changes to existing actions** (section 5 — these WILL break naive state | ||
| machines): single-asset listings, legacy-bundle auto-cancellation, execution-time | ||
| collection fee (applied at settlement, section 5.3). | ||
| collection fee (applied at settlement, section 4.3). |
Comment on lines
+36
to
+38
| let atomicmarket, atomicassets, token, delphi; | ||
| let author, seller, buyer, renter, renter2; | ||
| let founder1, founder2, temproy1, attrroy1, feesAtomic, marketowner; |
Comment on lines
+59
to
+62
| blockchain = new Blockchain(); | ||
| atomicassets = blockchain.createContract(AA, './tests/fixtures/atomicassets/atomicassets'); | ||
| atomicmarket = blockchain.createContract(MARKET, './build/atomicmarket'); | ||
| delphi = blockchain.createContract('delphioracle', './tests/fixtures/delphioracle/delphioracle'); |
Comment on lines
+69
to
+79
| author = blockchain.createAccount('author'); | ||
| seller = blockchain.createAccount('seller'); | ||
| buyer = blockchain.createAccount('buyer'); | ||
| renter = blockchain.createAccount('renter'); | ||
| renter2 = blockchain.createAccount('renter2'); | ||
| founder1 = blockchain.createAccount('founder1'); | ||
| founder2 = blockchain.createAccount('founder2'); | ||
| temproy1 = blockchain.createAccount('temproy1'); | ||
| attrroy1 = blockchain.createAccount('attrroy1'); | ||
| feesAtomic = blockchain.createAccount('fees.atomic'); | ||
| marketowner = blockchain.createAccount('marketowner'); |
|
|
||
| await atomicmarket.actions.addconftoken(['eosio.token', '8,WAX']).send(`${MARKET}@active`); | ||
|
|
||
| await mintTokens(token, 'WAX', 8, 1000000000, 10000, [buyer, renter, renter2]); |
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.
Why
We're descoping rentals from the V2 upgrade so the rest of V2 (royalty splits, single-asset listings, execution-time fee) can ship sooner, without having to get rentals right first. The AtomicAssets custodial primitive (
move/holders) is being removed in the companion PR atomicassets/atomicassets-contract#27.Rentals code is preserved:
archive/v2-custodial-rentalsbranch + thev2.0.0-rc1tag/pre-releaseexperiment/noncustodial-rentals(Experimental: non-custodial rental flow over AA lease primitives #11) + follow-ups (test+docs(rentals): review follow-ups (idempotent endrent, invalid-listing cancel, delphi extension, wiki rewrite) #12)What
announcerent/cancelrent/rentasset/endrent/payrentramand logslognewrent/logrentstart/logrentalrentalstable (+rentalendsindex, accessor) and the"rental"memo branch inreceive_asset_transferholders_s/get_holders()fromatomicassets-interface.hpp(no other caller)docs/wiki/Rentals.md; purge rental sections fromapi-integration.md, wiki pages, README, CLAUDE.mdrentalstable helper removed frommarket-smoke.test.js;auctions.test.jsonly loses its unusedrentalshelper (therenter/renter2accounts there are generic bidders and stay)Shared machinery is untouched:
internal_payout_sale,consume_counter/counters(sales + auctions still use them);migratenever seeded a rental counter.Base branch note
This PR is based on
v2/integration-rc1(3 commits ahead of master with the merged #10 Hydra→VeRT test migration), so merging it also lands those test commits on master — which was always the plan for the RC1 integration branch.Verification
make buildclean; 6 suites, 220 tests passing (228 minus the 8 rental tests)v2.0.0-rc1: exactly the 8 rental actions +rentalstable (and their structs) removed; nothing else changedNote on fixtures
tests/fixtures/atomicassets/still contains the rc3 AtomicAssets build (withmove/holders) — harmless superset; will be refreshed to the AA build without holders once atomicassets-contract#27 is released.