Skip to content

v2 · Feature 300 — Renting Assets (Version B) - #8

Closed
robrigo wants to merge 2 commits into
mainfrom
feat/v2-300-renting
Closed

v2 · Feature 300 — Renting Assets (Version B)#8
robrigo wants to merge 2 commits into
mainfrom
feat/v2-300-renting

Conversation

@robrigo

@robrigo robrigo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Asset renting via dual ownership (ownerholder) + move/logmove.


Mirrored into the canonical atomicassets org for the AtomicAssets v2 release + audit.
Original: wax-office-of-inspector-general/atomicassets-contract#10

⚠️ Hold — do not merge to master yet. Kept on a feature branch to avoid prod integration risk; pending the comprehensive v2 audit.

@robrigo
robrigo marked this pull request as ready for review June 2, 2026 20:49
@robrigo
robrigo requested a review from Copilot June 2, 2026 20:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces the first pieces of “dual ownership” support (separating owner vs holder) to enable asset renting-like behavior, by adding a move action and tracking holdership in a new holders table while keeping the existing transfer flow mostly intact.

Changes:

  • Added atomicassets::move(owner, from, to, asset_ids, memo) to move holdership under the true owner’s authorization.
  • Introduced a holders table (asset_id -> {holder, owner}) to persist holder state when owner != holder.
  • Updated internal_transfer to keep holders.owner in sync when ownership changes (and to drop the holders row when transferring to the holder).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/atomicassets.cpp Adds the move action and updates internal_transfer to reconcile holder rows during ownership transfers.
include/atomicassets.hpp Declares the new move action and adds the holders multi-index table.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/atomicassets.cpp
Comment on lines +83 to +87
/**
* 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
*/
Comment thread src/atomicassets.cpp
Comment on lines +1341 to +1352
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;
});
}
}
Comment thread src/atomicassets.cpp
Comment on lines +139 to +141
if (holders_itr != holders.end()){
check(holders_itr->holder == from,
("At least one asset invalidates the 'from:holder' constraint (ID: " + to_string(asset_id) + ")").c_str());
@robrigo

robrigo commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Closing: custodial rentals were descoped from the v2 release train, so this feature is deliberately not on main and was never part of rc4 or rc5. The decision was to let the rest of v2 ship without settling rentals first.

Nothing is lost. The work is preserved on two branches, both kept indefinitely:

Whether and how rentals return has not been decided. The non-custodial approach is being explored separately in #26. Closing this PR records the descope; it does not reject the work.

@robrigo robrigo closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants