Skip to content

feat(wseg-build): full AtomicAssets shape — v2 writer (Cycle B)#10

Merged
igorls merged 4 commits into
mainfrom
feat/aa-cycleb-full-shape
Jun 7, 2026
Merged

feat(wseg-build): full AtomicAssets shape — v2 writer (Cycle B)#10
igorls merged 4 commits into
mainfrom
feat/aa-cycleb-full-shape

Conversation

@igorls
Copy link
Copy Markdown
Member

@igorls igorls commented Jun 7, 2026

Cycle B — the writer half. Pairs with wormdb-domain-atomicassets#3 (the v2 reader); they're byte-locked via ASSET_VERSION 1→2 and should be reviewed/merged together (a v2 reader can't read a v1 segment and vice-versa; the deploy rebuilds the segment from Mongo).

What it writes

  • encode_template +transferable/burnable/max_supply(u32)/issued_supply(u32) → TemplateRec.
  • New encode_collection/TABLE_AA_COLL_FWD: version|collection|author|allow_notify|auth_count|authorized|notify_count|notify|market_fee(f64)|data attrs. market_fee is the contract double (f64, no precision loss).
  • aa_builder: push_collection (Mongo atomicassets-collections → COLL_FWD, data keyed by config collection_format); push_template reads the 4 new fields; streams collections after config.
  • Cross-repo byte goldens golden_template_v2 / golden_collection_v2 (mirrored on the reader side).

Design calls (data-verified)

  • created_at_* is NOT written — the snapshot's block_num is the snapshot block (uniform across all rows, confirmed via Mongo), not per-entity creation. Those are history-tier (Phase 3).
  • market_fee = f64 (the contract's double), not f32 — avoids parity-diff precision loss.

Verification

  • 31 tests (template/collection round-trips + cross-repo byte goldens); clippy --release --all-targets -D warnings + fmt clean.
  • Built the Jungle 4 segment: 31 collections, 188 v2 templates; the served shape matches the Mongo source docs field-by-field (see the reader PR).

…(Cycle B)

Carry the fields the full eosio-contract-api response needs. ASSET_VERSION 1->2
(lockstep with the wormdb-domain-atomicassets reader; forces a segment rebuild).

- aa_binfmt.rs: encode_template +transferable/burnable/max_supply(u32)/issued_supply(u32)
  (TemplateRec); new encode_collection/decode_collection (TABLE_AA_COLL_FWD): version|
  collection|author|allow_notify|auth_count|authorized|notify_count|notify|market_fee(f64,
  the contract double)|data attrs; cross-repo byte goldens golden_template_v2 / golden_collection_v2.
- aa_builder.rs: push_collection (Mongo atomicassets-collections -> COLL_FWD), keying
  collection data attrs by the config collection_format; push_template reads the 4 new
  fields; finish() emits TABLE_AA_COLL_FWD; warns if an account list exceeds the 255 cap.
- bin/aa_build.rs: streams collections after config (collection_format index dependency);
  adds transferable/burnable/max_supply/issued_supply to the template projection.
- aa_live.rs: compaction reuses decode_template's struct (+the new fields).

created_at_* deliberately NOT written: the snapshot's block_num is the snapshot block
(uniform across rows), not per-entity creation — those are history-tier (Phase 3).

Tests: 31 pass (incl. template/collection round-trips + cross-repo byte goldens);
clippy + fmt clean. Built the Jungle 4 segment: 31 collections, 188 v2 templates.

Pairs with wormdb-domain-atomicassets (the v2 reader) — review/merge together.
Copilot AI review requested due to automatic review settings June 7, 2026 01:12
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request upgrades the asset binary format to version 2, adding new fields (transferable, burnable, max_supply, issued_supply) to template forward records and introducing support for collection forward records (TABLE_AA_COLL_FWD). The reviewer feedback highlights minor issues in vector capacity calculations for both encode_template and encode_collection which could lead to immediate reallocations due to missing attribute count prefixes. Additionally, it is recommended to safely cast indices to u8 when building coll_format_idx to prevent silent overflow if the collection format contains more than 256 fields.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/wseg-build/src/aa_binfmt.rs Outdated
issued_supply: u32,
immutable: &[Attr],
) -> Vec<u8> {
let mut o = Vec::with_capacity(23 + immutable.len() * 12);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The capacity calculation 23 + immutable.len() * 12 under-allocates the vector. The fixed fields take 23 bytes (1 for version, 4 for template_id, 8 for schema, 1 for transferable, 1 for burnable, 4 for max_supply, and 4 for issued_supply). However, put_attrs also writes a 2-byte attribute count prefix (attrs.len() as u16). Therefore, the fixed overhead is 25 bytes, and the capacity should be initialized with 25 + immutable.len() * 12 to avoid an immediate reallocation when writing attributes.

Suggested change
let mut o = Vec::with_capacity(23 + immutable.len() * 12);
let mut o = Vec::with_capacity(25 + immutable.len() * 12);

Comment thread crates/wseg-build/src/aa_binfmt.rs Outdated
) -> Vec<u8> {
let nauth = authorized.len().min(255);
let nnotify = notify.len().min(255);
let mut o = Vec::with_capacity(27 + (nauth + nnotify) * 8 + data.len() * 12);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The capacity calculation 27 + (nauth + nnotify) * 8 + data.len() * 12 under-allocates the vector. The fixed fields take 28 bytes (1 for version, 8 for collection, 8 for author, 1 for allow_notify, 1 for nauth, 1 for nnotify, and 8 for market_fee). Additionally, put_attrs writes a 2-byte attribute count prefix. Therefore, the fixed overhead is 30 bytes, and the capacity should be initialized with 30 + (nauth + nnotify) * 8 + data.len() * 12 to avoid an immediate reallocation.

Suggested change
let mut o = Vec::with_capacity(27 + (nauth + nnotify) * 8 + data.len() * 12);
let mut o = Vec::with_capacity(30 + (nauth + nnotify) * 8 + data.len() * 12);

Comment on lines +135 to +139
self.coll_format_idx = fmt
.iter()
.enumerate()
.map(|(i, (n, _))| (n.clone(), i as u8))
.collect();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If fmt contains more than 256 fields, casting i as u8 will silently overflow and wrap around, leading to duplicate or incorrect indices in coll_format_idx. To prevent silent corruption, use u8::try_from(i).expect(...) to safely check for overflow.

        self.coll_format_idx = fmt
            .iter()
            .enumerate()
            .map(|(i, (n, _))| {
                let idx = u8::try_from(i).expect("collection_format has too many fields (max 256)");
                (n.clone(), idx)
            })
            .collect();

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 082a863595

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

self.base.for_each_entry(TABLE_AA_SCHEMAS, |key, blob| {
b.push_schema_raw(key, &decode_schema_format(blob));
});
self.base.for_each_entry(TABLE_AA_TMPL_FWD, |_k, blob| {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve collection forward records during compaction

With the new builder path now populating TABLE_AA_COLL_FWD, this compaction carry-over only re-emits schemas/templates before folding assets and calling finish(). In a live server that compacts a v2 segment, the rebuilt segment will therefore have an empty collection forward table (stats.collections == 0), so collection metadata such as author, notify accounts, market fee, and collection data disappears after the first compaction even though the source base contained it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds the AtomicAssets v2 writer shape to wseg-build, expanding what gets emitted into the .wseg segment so it matches the full API surface (templates gain v2 fields; collections gain a new forward table), and bumps the on-blob version byte to ASSET_VERSION = 2 for cross-repo byte-locking with the reader.

Changes:

  • Bumps ASSET_VERSION to 2 and extends the template FWD blob layout to include transferable, burnable, max_supply, and issued_supply.
  • Introduces collection FWD encoding/decoding (encode_collection / decode_collection) and streams atomicassets-collections into the segment build.
  • Updates builder + live compaction paths to use the new template shape (but compaction currently does not carry over collections).

Reviewed changes

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

File Description
crates/wseg-build/src/bin/aa_build.rs Streams collections before schemas/templates and reports collections in output stats.
crates/wseg-build/src/aa_live.rs Updates compaction to re-emit v2 template records (but currently drops the new collection FWD table).
crates/wseg-build/src/aa_builder.rs Builder support for collection FWD emission + v2 template fields; adds helpers for bool/f64/name arrays.
crates/wseg-build/src/aa_binfmt.rs Bumps version byte to v2; updates template blob layout and adds collection blob codec + byte-golden tests.

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

Comment on lines 1104 to 1109
let mut b = AtomicBuilder::new(self.facet_fields.clone());
// schemas + templates carry straight over from the base (overlay add/extend would also fold here)
self.base.for_each_entry(TABLE_AA_SCHEMAS, |key, blob| {
b.push_schema_raw(key, &decode_schema_format(blob));
});
self.base.for_each_entry(TABLE_AA_TMPL_FWD, |_k, blob| {
Comment on lines +402 to +406
pub fn decode_template(b: &[u8]) -> TemplateRec {
let mut p = 1usize; // skip version
let template_id = gi32(b, &mut p);
let schema = gu64(b, &mut p);
let transferable = b[p];
Comment on lines +473 to +476
pub fn decode_collection(b: &[u8]) -> CollectionRec {
let mut p = 1usize; // skip version
let collection = gu64(b, &mut p);
let author = gu64(b, &mut p);
- encode_template / encode_collection: correct the Vec::with_capacity hints to
  include the attr-count prefix (25 = 23 header + 2; 30 = 28 fixed + 2) — a hint,
  so no behavior change, just avoids a reallocation.
- push_config: guard the collection_format index against a silent `as u8` wrap
  (u8::try_from + filter_map) — field_idx is a u8 so fields past 255 are
  unrepresentable and skipped (never happens for a real collection_format).

Bytes unchanged (goldens still pass); 31 tests + clippy + fmt clean.
@igorls
Copy link
Copy Markdown
Member Author

igorls commented Jun 7, 2026

Addressed (commit 8c04b73):

  • encode_template / encode_collection capacity hints — corrected to include the 2-byte attr-count prefix (25 / 30). These are with_capacity hints, so it's a micro-opt (one fewer realloc), not a correctness issue; the emitted bytes are unchanged (goldens still pass).
  • coll_format_idx as u8 overflow — now u8::try_from(...).ok() + filter_map, so a (hypothetical) collection_format with >255 fields skips the unrepresentable indices instead of silently wrapping. Real collection_formats carry ~5 fields, far below the cap.

31 tests + clippy + fmt clean.

- aa_live compaction: carry TABLE_AA_COLL_FWD over into the rebuilt segment
  (new push_collection_raw) — Cycle B added collections to the builder, so without
  this, compaction silently dropped every collection forward record.
- decode_template / decode_collection: debug_assert the version byte so a v1/corrupt
  blob fails with a clear "version byte mismatch" instead of a confusing bounds panic.
- coll_format_idx already guards the `as u8` wrap with u8::try_from (prior commit).

31 tests + clippy + fmt clean.
@igorls
Copy link
Copy Markdown
Member Author

igorls commented Jun 7, 2026

Second pass addressed (commit 537053c):

  • aa_live compaction dropped collections (Codex P1 + Copilot) — added push_collection_raw and a TABLE_AA_COLL_FWD carry-over in compact_with, so compaction re-emits collection forward records (stats now count them).
  • decode_template/decode_collection version validation (Copilot) — debug_assert_eq!(b[0], ASSET_VERSION, …) turns a v1/corrupt blob into a clear version-mismatch panic instead of a confusing bounds error.
  • coll_format_idx as u8 wrap (Gemini, re-posted) — already guarded with u8::try_from(i).ok() in 8c04b73; >255 fields are skipped (never happens for a real collection_format).

31 tests + clippy + fmt clean.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 537053ca3c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/wseg-build/src/aa_binfmt.rs Outdated
Comment on lines +403 to +406
debug_assert_eq!(
b[0], ASSET_VERSION,
"decode_template: version byte mismatch (rebuild the segment)"
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce template version checks in release builds

When a release build compacts or otherwise carries over a pre-v2 segment, this debug_assert_eq! is compiled out, so the v2 decoder proceeds to interpret the old v1 template layout as transferable/burnable/supply fields before the attr list. In that upgrade scenario the code can panic later on bounds checks or, for unlucky bytes, re-emit corrupted template metadata instead of failing closed as the version bump comment requires; use a runtime check/Result or explicit panic before decoding.

Useful? React with 👍 / 👎.

#10 review)

decode_template / decode_collection used debug_assert_eq! for the version byte,
which is compiled out in release — so a release compaction over a pre-v2 base
would mis-parse it. Promote to assert_eq! so a version mismatch fails loud
(these decode ~hundreds of templates/collections, not the per-asset hot path).
@igorls
Copy link
Copy Markdown
Member Author

igorls commented Jun 7, 2026

Addressed (commit 8234b34): promoted the decode_template/decode_collection version checks from debug_assert_eq! to assert_eq!, so a version mismatch fails loud in release too (e.g. a release compaction over a pre-v2 base) instead of mis-parsing. These run per-template/collection (~hundreds), not the per-asset hot path, so the cost is negligible.

@igorls igorls merged commit cc6b53e into main Jun 7, 2026
1 check passed
@igorls igorls deleted the feat/aa-cycleb-full-shape branch June 7, 2026 01:45
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