v2 · Integration — Features 1-8 + 300 (dev-production-3) - #9
Merged
Conversation
…le templates & erases the entry
…vent inconsistencies with authorswaps
… for check_as_collection_auth & notify_collection_accounts, removed coldata_cleanup related(), string constructing optimizations for frequent actions' checks, renamed mutable templates table to templates2
…ifications to owner, from or to
There was a problem hiding this comment.
Pull request overview
This PR is an integrated AtomicAssets v2 contract surface update, adding new authoring/templating capabilities and a “holdership” concept while refactoring table access and notifications/auth checks for performance.
Changes:
- Added new on-chain features: asset holdership via
move+holderstable, author swap offers (createauswap/acceptauswap/rejectauswap), schema type metadata (setschematyp), and mutable template data support (createtempl2/settempldata/deltemplate/redtemplmax). - Deprecated native token backing paths (
mintassetnow rejectstokens_to_back;backassethard-fails; removedinternal_back_assetusage). - Refactored table access to use
get_*()fetch helpers and introduced a low-level partial row read for collection auth/notify lookups.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/atomicassets.cpp |
Implements new v2 actions/features, deprecates backing, updates transfer logic for holdership, and adds low-level collection partial reads. |
include/checkformat.hpp |
Adjusts format validation to pass by reference. |
include/atomicdata.hpp |
Adds FORMAT_TYPE to support schema type metadata. |
include/atomicassets.hpp |
Extends contract ABI (new actions/tables), adds table fetch helpers, and updates internal function signatures. |
include/atomicassets-interface.hpp |
Updates the external interface header to mirror new v2 tables/types and helper accessors. |
Comments suppressed due to low confidence (1)
src/atomicassets.cpp:247
- createcol() doesn't enforce the 24-element caps for authorized_accounts/notify_accounts that later code (partial_read_collection) relies on. A collection created with >24 entries can make partial_read_collection read too few bytes and cause unpack failures when checking auth / sending notifications.
auto config = get_config();
config_s current_config = config.get();
collections.emplace(author, [&](auto &_collection) {
_collection.collection_name = collection_name;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nto integration Consolidates the full AtomicAssets v2 test suite onto feat/v2-integration so the integration line is self-testable (make build && npx jest). feat/v2-integration is an ancestor of the test branch, so this only adds the 16 test commits + Makefile. Excludes PR #13 gap-fill (its byte-math suite asserts the PR #12 fix, which is held).
…ild) CDT 4.1 -abigen changed two wire-compatible but tool-visible ABI spellings vs the CDT used to compile the deployed AtomicAssets contract: 1. std::pair fields first/second (was key/value) 2. std::vector<uint8_t> bytes (was uint8[]) Both break existing integrations (atomicassets-js, wallets, ECA indexer, cleos JSON) that pack/unpack ATTRIBUTE_MAP and serialized_data by these names, though the compiled wasm and binary wire format are identical. scripts/patch-abi.py restores the deployed spellings. It is applied ONLY by the new `make release` target (build + patch) for distribution / cleos set contract; the VeRT suite is written against the raw CDT 4.1 abi and runs on `make build`, so the patch is deliberately kept out of the test path. Verified: after patching, the rc abi's shared surface is byte-for-byte equivalent to on-chain get_abi (only additive v2 features differ); wasm sha256 unchanged.
The atomicassets-contract repo had no CI. This adds a GitHub Actions workflow that installs the pinned CDT (4.1.1, public .deb), builds the contract (make build), and runs the @vaulta/vert jest suite on pushes to the v2 lines + master and on all PRs. Runs on ubuntu-latest (the FACINGS monorepo's self-hosted + private-GHCR CI is not reachable from this public org). A harness guard skips cleanly on refs without the test harness (e.g. master before v2 lands), so the same workflow can live on master to gate its PRs/pushes without failing until the suite arrives. Actions are SHA-pinned.
Contributor
Author
|
Attribution note for merging v2 to
|
…> FACINGS); AtomicAssets is the open standard, AtomicHub the marketplace
Fix/interface header get self
The interface header is consumed by external contracts (atomicpacks, atomictools, atomicbridge, ...), never by atomicassets itself, so the main VeRT suite cannot catch regressions in it. The v2 get_self() anchoring bug (fixed in #21) shipped exactly because nothing in CI ever compiled the header from a consumer's context — get_self() is a member of eosio::contract, so the v2 header did not even compile for consumers. Add a minimal consumer fixture (tests/fixtures/interface-consumer) built by `make build` and deployed at `ifaceconsumr` in a new VeRT suite: - the compile alone catches the get_self() class of regression - the runtime tests catch any compilable-but-wrong-anchor variant: reads of collections (contract scope), assets (per-owner scope), and templates (per-collection scope) through the header from a non-atomicassets account, plus a negative control proving the checks execute Verified red/green: with the pre-#21 header restored, the consumer build fails; with the fixed header, 4/4 tests pass.
test: compile + read atomicassets-interface.hpp from a consumer contract
addcolauth and addnotifyacc cap each vector at 24, but createcol wrote both verbatim with no size check. That let a single createcol seed a collection past the cap and overflow partial_read_collection's fixed read window (auth path once authorized > ~39; notify path once authorized+notify > 63), bricking check_has_collection_auth / notify_collection_accounts for that collection. This is the residual gap noted in #12, which fixes the read side but cannot bound an unbounded write. Add the same <=24 checks createcol was missing (identical messages to addcolauth/addnotifyacc) plus VeRT cases: createcol with 25 authorized or 25 notify throws, and the 24+24 boundary succeeds. Full suite green (cdt-cpp 4.1.1). Note: does not retroactively touch existing >24 collections; WAX mainnet has 0 collections above 24 authorized (max 23) and 1 with >24 notify (31), all well within the post-#12 read budgets.
…y_accounts The authorized-accounts path (type=false, 330-byte db_get_i64 buffer) deserialized notify_accounts unconditionally, reading past the truncated buffer once a collection's authorized+notify accounts exceeded ~38 -> 'datastream attempted to read past the end', bricking every check_has_collection_auth caller (createschema/createtempl/setschematyp/ extendschema/...). Root cause: the byte-math budget assumed a phantom +128 row/PK prefix that db_get_i64 does not return (it returns payload only), and the auth path was sized to end-of-authorized (210B) while the code read to end-of-notify (403B). Fix: early-return after authorized_accounts on the auth path (<=210B, within 330); the notify path still reads through notify_accounts (<=403B, within 523). Behavior-preserving for all previously-succeeding inputs; only un-bricks the previously-throwing auth case. Correct the misleading byte-math comments. Closes audit A-BYTEMATH (the A.7 'safe' verdict was wrong: verified vs eosio.cdt 4.1.1, reproduces on mainnet).
… over-read Adds tests/Collection Actions/partial-read-collection-bytemath.test.js covering the auth path at 19+19, the former 20+20 overflow boundary, and the max 24+24 authorized+notify caps, plus a 24-account notify-path read past a large serialized_data. All four pass on the fixed contract; the 20+20 and 24+24 cases throw 'datastream attempted to read past the end' on an unfixed build, so the file fails the suite without the fix (real regression guard). Validated locally with cdt-cpp 4.1.1 (make build + npx jest).
…bytemath fix(atomicassets): partial_read_collection auth path over-reads notify_accounts (pre-mainnet blocker)
fix(atomicassets): cap createcol authorized/notify accounts at 24 (follow-up to #12)
The VeRT test dirs used Title Case with spaces (tests/Collection Actions/, etc.), which is shell-hostile (needs quoting everywhere) and non-idiomatic. Rename all nine to kebab-case (tests/collection-actions/, ...). The jest glob is **/*.test.js so this is a pure rename: no config change, full suite still green (42 suites). Also updates the two stale path references in the Makefile and interface-consumer fixture comments. fixtures/ kept as-is.
chore(tests): rename test directories to kebab-case
Replace em dashes with plain punctuation across the Makefile, patch-abi.py, and a few test comments. No code or behavior change; the deployed source (src, include, README) was already clean.
chore: drop em dashes from comments
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).
feat!: remove custodial rental primitives (move/logmove/holders) from V2
This branch and the default branch each added the workflow independently, so the differing branch filters collide as an add/add conflict on merge. Matching the default branch resolves it and keeps one copy of the file after the merge.
…ater check() evaluates both arguments, so the remaining-seconds message was built on every successful accept, and acceptance_date - now underflowed as uint32 to produce a string that was then discarded: wasted CPU in a release whose point is CPU cost. The strict comparison also rejected the acceptance_date second itself, so a swap became acceptable a second after the date it advertises. Both now sit behind an explicit branch, and a regression test pins the boundary by failing against the previous logic. check_format took a non-const reference while never writing through it, leaving callers unable to pass a const format and readers unsure whether it mutated. The docstring claimed three weeks of validity where the code allows two from creation under active permissions and one under owner. The test-directory listings named directories that no longer exist, and two suites located inline actions by their position in the trace list rather than by name, so unrelated changes to inline dispatch would fail them.
This was referenced Jul 16, 2026
Closed
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.
Full integrated v2 surface — the reference branch for the complete contract.
Mirrored into the canonical
atomicassetsorg for the AtomicAssets v2 release + audit.Original: wax-office-of-inspector-general/atomicassets-contract#12
masteryet. Kept on a feature branch to avoid prod integration risk; pending the comprehensive v2 audit.