Skip to content

feat!: remove custodial rental primitives (move/logmove/holders) from V2 - #27

Merged
robrigo merged 1 commit into
feat/v2-integrationfrom
feat/remove-custodial-rentals
Jul 3, 2026
Merged

feat!: remove custodial rental primitives (move/logmove/holders) from V2#27
robrigo merged 1 commit into
feat/v2-integrationfrom
feat/remove-custodial-rentals

Conversation

@robrigo

@robrigo robrigo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Why

We're descoping rentals from the V2 upgrade so the rest of V2 can ship sooner, without having to get rentals right first. The custodial dual-ownership mechanism (move/logmove + holders table) only existed to support AtomicMarket rentals, which are being removed from AtomicMarket V2 in a companion PR.

Rentals code is preserved:

What

  • Remove move action and logmove notification
  • Remove holders table, holders_s struct, and get_holders() from both atomicassets.hpp and the consumer-facing atomicassets-interface.hpp
  • Remove the holder-erase block from burnasset
  • Remove holder bookkeeping from internal_transfer

No other V2 feature (templates2, schematypes, deltemplate/redtemplmax, author succession, byte-math) reads holder state — this is a pure excision.

Verification

  • make build clean
  • ABI diff vs v2.0.0-rc3: exactly move, logmove (actions/structs) and holders/holders_s (table/struct) removed; nothing else changed
  • Tests: deleted move.test.js (16) and renting-invariants.test.js (3); removed holder-specific cases from transfer.test.js (4) and burnasset.test.js (1). Suite: 38 suites, 313 passing (1 pre-existing skip)

Deployment note

rc3 is live on jungle4 + wax-testnet. Before deploying the rc4 build, on-chain holders rows (from e2e runs) must be cleared — after the flip there is no action left that can erase them. That cleanup is tracked on our side.

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 removes the custodial rental/dual-ownership primitives from AtomicAssets V2 (the move/logmove flow and the holders table), aligning with the V2 scope reduction so non-rental V2 features can ship independently.

Changes:

  • Removed the move action and logmove notification action from the contract implementation and ABI headers.
  • Removed the holders table/struct and all holder bookkeeping from burnasset and internal_transfer.
  • Removed/updated test coverage that specifically exercised custodial holder behavior (move, holders-dependent transfer/burn invariants).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/atomicassets.cpp Removes move/logmove actions and deletes all holders-related logic in burn/transfer flows.
include/atomicassets.hpp Removes move/logmove action declarations plus holders table definition and get_holders() helper.
include/atomicassets-interface.hpp Removes consumer-facing holders table/struct and get_holders() accessor.
tests/Transfer-Offer Actions/transfer.test.js Removes holder-specific transfer test cases that depended on move/holders.
tests/Deposit-Withdraw-Back-Burn Actions/burnasset.test.js Removes holder-specific burn test case that depended on move/holders.
tests/Asset Actions/renting-invariants.test.js Deletes characterization tests for legacy “held asset” burn/transfer behavior.
tests/Asset Actions/move.test.js Deletes move action test suite since the action is removed.

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

Custodial rentals are descoped from the V2 release so the rest of V2 can
ship without them. The dual-ownership mechanism is removed in full:

- move action and logmove notification
- holders table (+ holders_s struct, get_holders accessor) in both the
  contract header and the consumer-facing interface header
- holder-erase block in burnasset
- holder bookkeeping in internal_transfer

No other V2 feature reads holder state, so this is a pure excision.
ABI diff vs v2.0.0-rc3: exactly move, logmove, holders(_s) removed.

Rentals live on: the custodial implementation is preserved on
archive/v2-custodial-rentals (and the v2.0.0-rc1..rc3 tags); the
non-custodial rework continues on experiment/noncustodial-rentals (#26).

Tests: move.test.js (16) and renting-invariants.test.js (3) deleted;
holder-specific cases removed from transfer.test.js and burnasset.test.js.
Suite: 40 suites, 324 passing (1 pre-existing skip).
@robrigo
robrigo force-pushed the feat/remove-custodial-rentals branch from 65f8dab to de520c4 Compare July 3, 2026 00:01
@robrigo
robrigo merged commit ce87cb2 into feat/v2-integration Jul 3, 2026
1 check passed
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.

2 participants