Fix/ip registry 807 810 - #938
Open
Zalgia-Saga wants to merge 4 commits into
Open
Conversation
…flict FIXMEs contracts/ip_registry/src/test.rs was already active (mod test; uncommented) and structurally sound, but two stale FIXME comments still claimed it had merge-conflict compile errors. Removed the noise and added a CI grep step that fails the build if any *.rs file still carries an unresolved "merge conflict" FIXME outside the known, still-disabled benchmarks.rs and invariant_tests.rs modules. Closes AtomicIP#807
…tions IpRecord already tracked expiry_timestamp and grace_period_seconds, but no event topic existed for expiry transitions (only REVOKE_TOPIC, TRANSFER_TOPIC, and BATCH_VERIFY_TOPIC were defined), so off-chain indexers had no cheap way to detect an IP crossing into its grace period or expiring. Adds EXPIRY_TOPIC and emits it from verify_commitment the first time it observes an IP past its expiry_timestamp, using a per-IP ExpiryNotified flag so the event fires exactly once per transition. The flag is cleared whenever set_ip_expiry or renew_ip_commitment establishes a new expiry, so a subsequent expiry can notify again. Closes AtomicIP#808
OwnershipShare.percentage was documented as "sum of all should be 100" but nothing enforced it, and add_co_owner/remove_co_owner didn't track percentages at all (the type was unused dead code). Adds require_valid_share_total in validation.rs, which sums a cap table's percentages and panics with the new InvalidShareTotal error if they don't total exactly 100. add_co_owner now takes an explicit percentage, deducts it from the owner's remaining share, and validates the resulting cap table; remove_co_owner returns the departing co-owner's percentage to the owner and re-validates. Both paths persist the cap table under a new DataKey::OwnershipShares(ip_id), readable via the new get_ownership_shares. Closes AtomicIP#809
…d IpVersionChain DataKey::IpVersions(u64) (direct child version IDs per node) and DataKey::IpVersionChain(u64) (full chain rooted at a given IP) are both written together in create_ip_version, but nothing verified they stayed in agreement, risking silent drift if a future change touched one without the other. Added a differential test that builds a branching version tree (versions created off both the root and a non-root version) and, after every single versioning call, reconstructs the version set by walking IpVersions from the root and asserts it exactly matches the flattened IpVersionChain. Audited the two writers: both are only ever updated together inside create_ip_version, so no divergence exists today. Left IpVersions as its own stored key rather than deriving it from IpVersionChain — get_ip_versions() needs O(1) direct-children lookups for a given node, which a flattened, unordered-by-parent chain can't provide without an extra scan. Closes AtomicIP#810
|
@Zalgia-Saga Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
1 participant
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.
Summary
Fixes four issues in the ip_registry contract: stale FIXME noise in test.rs, a missing expiry event topic, an unenforced ownership-percentage invariant, and a missing consistency check between two version-tracking storage keys.
contracts/ip_registry/src/test.rs was already fine (#807)
The two FIXME comments claimed merge-conflict compile errors, but mod test; was already active and the file is structurally sound (balanced braces, every ContractError/type it references exists, and the two tests that call not-yet-implemented entrypoints are correctly marked #[ignore]). Removed the stale comments and added a CI step that greps for any future FIXME.*merge conflict so this can't quietly go stale again (scoped to exclude benchmarks.rs/invariant_tests.rs, which are still intentionally disabled and out of scope here).
EXPIRY_TOPIC (#808)
Added EXPIRY_TOPIC and emit it from verify_commitment the first time an IP is observed past its expiry_timestamp. A per-IP ExpiryNotified flag ensures it fires exactly once per transition, and is cleared by set_ip_expiry/renew_ip_commitment so a later expiry can notify again.
OwnershipShare sum invariant (#809)
add_co_owner/remove_co_owner never tracked percentages at all — OwnershipShare was dead code. Added require_valid_share_total (validation.rs) and a new InvalidShareTotal error; add_co_owner now takes a percentage, deducts it from the owner's remaining share, and validates the cap table; remove_co_owner returns the share to the owner. New get_ownership_shares getter added to make the cap table readable.
IpVersions / IpVersionChain cross-check (#810)
Audited both writers — they're only ever updated together inside create_ip_version, so there's no divergence today. Added a differential test that builds a branching version tree and asserts the two representations agree after every versioning call. Kept them as separate keys rather than deriving one from the other, since get_ip_versions needs O(1) direct-child lookups that a flattened chain can't give without a scan.
Test plan
cargo test -p ip_registry (no cargo toolchain was available in this environment to run it directly — please run in CI)
Confirm the new CI "stale merge-conflict FIXME" step passes
Spot-check test_verify_commitment_emits_expiry_event_exactly_once, ownership_share_tests::*, and differential_ip_versions_and_chain_agree_across_branching_tree
Closes #807
Closes #808
Closes #809
Closes #810