Skip to content

WIRE-439: restrict node-owner claims to new accounts #767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshglogau wants to merge 8 commits into
base: master
Choose a base branch
from
+224 −250
Draft

WIRE-439: restrict node-owner claims to new accounts #767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e7aa863
fix(roa): restrict NFT registration to new accounts (WIRE-439)
joshglogau Nov 11, 2026
d8fe120
WIRE-439: use supported authority-key access in regressions
joshglogau Nov 11, 2026
8d13791
WIRE-439: rebuild ROA and MSGCH contract artifacts
joshglogau Nov 11, 2026
9c1a704
WIRE-439: compare shared keys without stream formatting
joshglogau Nov 11, 2026
01edf6e
WIRE-439: protect template comma in Boost assertion
joshglogau Nov 11, 2026
c622b5f
WIRE-439: retain existing-account fixtures for unrelated dispatch paths
joshglogau Nov 11, 2026
ad43ac7
WIRE-439: account for bootstrap AuthX linkage in dispatch regression
joshglogau Nov 11, 2026
c0a27c9
WIRE-439: align ROA comments with atomic registration
joshglogau Nov 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,8 @@ cd $BUILD_DIR && python3 tests/<test_name>.py
```
Do NOT run from the source root — that would use stale or missing binaries.

Review [contract upgrade order](docs/contract-upgrade-order.md) when changing inline calls between system contracts; unchanged ABIs do not guarantee mixed-version compatibility.

## Smart Contract Compilation (System Contracts specifically)

The system contracts are compiled via the `contracts_project` CMake target.
Expand Down
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
Wire Sysio is a fork of Spring, a C++ implementation of the [AntelopeIO](https://github.com/AntelopeIO) protocol. It
contains blockchain node software and supporting tools for developers and node operators.

Resource allocation and node-owner registration are described in [the ROA overview](docs/roa-overview.md).

## Branches

The `master` branch is the latest stable branch.
Expand Down
17 changes: 5 additions & 12 deletions contracts/sysio.msgch/src/sysio.msgch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,16 +585,13 @@ std::optional<em_identity> em_identity_from_uncompressed_key(const std::vector<c
};
}

/// Decode an inbound NodeOwnerRegistration attestation and drive the NFT node-owner claim: create
/// the vanity-named Wire account from the claim's wire_pub_key, then register the owner and record
/// the depositor's ETH link. Both steps are inline-sent to sysio.roa declaring {sysio.roa, active}
/// (permitted because sysio.msgch is privileged; no cross-contract active grant is required),
/// newnameduser first so any newaccount runs depth-first before nodeownreg. Invalid names and full
/// tiers skip creation; nodeownreg records the rejection without allocating an account.
/// Decode an inbound NodeOwnerRegistration and send one claim to sysio.roa::nodeownreg.
/// ROA rejects existing accounts before creating the chosen name, allocating the tier and
/// recording the depositor's ETH link atomically. Privileged msgch declares {sysio.roa, active}.
///
/// Trust-OPP: a malformed envelope (undecodable proto, unparseable name, unusable key bytes,
/// out-of-range tier) is silently dropped here -- nothing is sent. A well-formed envelope whose
/// *claim* is rejected (name wrong length for tier, account held by a different key, already registered,
/// *claim* is rejected (name wrong length for tier, account already exists, already registered,
/// tier at capacity)
/// is soft-failed inside nodeownreg, which records a REJECTED audit row. Neither path aborts the
/// dispatching transaction.
Expand Down Expand Up @@ -635,11 +632,7 @@ void dispatch_node_owner_reg(const std::vector<char>& data, uint64_t chain_code)
auto eth_identity = em_identity_from_uncompressed_key(reg.actor_pub_key);
if (!eth_identity) return; // unusable depositor ETH key; drop

// 1) Create the account (idempotent; soft-skips a name that breaks the tier rule).
action(permission_level{ROA_ACCOUNT, "active"_n}, ROA_ACCOUNT, "newnameduser"_n,
std::make_tuple(*owner, *wire_pk, tier)).send();

// 2) Register the owner + record the ETH link, with claim-payload soft-fail + audit recording.
// ROA owns the nonexistence check and account creation within the same claim.
action(permission_level{ROA_ACCOUNT, "active"_n}, ROA_ACCOUNT, "nodeownreg"_n,
std::make_tuple(*owner, tier, eth_identity->key, *wire_pk, eth_identity->address)).send();
}
Expand Down
Binary file modified contracts/sysio.msgch/sysio.msgch.wasm
View file
Open in desktop
Binary file not shown.
93 changes: 23 additions & 70 deletions contracts/sysio.roa/sysio.roa.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <sysio.opp.common/evm_address.hpp>
#include <sysio.opp.common/secp256k1_point.hpp>
#include <sysio.opp.common/safe_ops.hpp> // add_sat_u64 / add_sat_i64 -- never-throw saturating accumulators
#include <sysio/permission.hpp> // get_permission -- read an account's active authority in nodeownreg

#include <algorithm>
#include <limits>
Expand All @@ -19,6 +18,8 @@ namespace sysio {
// literals -- a contract rename is one change here, not scattered across call sites).
constexpr name AUTHEX_ACCOUNT = "sysio.authex"_n;
constexpr name AUTHEX_RECORDLINK = "recordlink"_n;
/// Existing privileged allocator, queued after the claim creates its account.
constexpr name ROA_FORCEREG = "forcereg"_n;

/// Names under this prefix belong to system accounts. The chain refuses them only to non-privileged
/// creators, and sysio.roa is privileged, so node-owner claims must refuse them here.
Expand Down Expand Up @@ -764,13 +765,8 @@ namespace sysio {
void roa::nodeownreg(const name& owner, const uint8_t& tier, const public_key& eth_pub_key,
const public_key& wire_pub_key,
const bytes& eth_address) {
// Dispatched by the OPP depot (sysio.msgch) when it processes an inbound
// ATTESTATION_TYPE_NODE_OWNER_REG attestation. msgch inline-sends newnameduser (account
// create) and then this action, both declaring permission_level{sysio.roa, active}.
// Privileged sysio.msgch may declare that target permission without a cross-contract active
// grant, so deployment must preserve msgch's privileged status. Inline actions run
// depth-first, so any newaccount has executed before this runs. Invalid names and full
// tiers skip creation; this action records their rejection instead.
// One authenticated depot claim owns creation and registration. Existing accounts are
// never eligible, even when the payload copies their public active key (WIRE-439).

@heifner heifner Nov 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please drop the ticket ID; the sentence already states the rule. Same for "under WIRE-439" above forcereg_reconciles_existing_reslimit.

require_auth(get_self());

// ---- Envelope / system invariants (depot misuse) ----
Expand Down Expand Up @@ -798,48 +794,28 @@ namespace sysio {

// ---- Claim-payload checks (trust-OPP soft-fail) ----
// The OPP envelope can be well-formed while the claim inside it is bad (the name the user
// chose is invalid, names an account they do not control, or is a replay). Aborting the
// chose is invalid, names an existing account, or is a replay). Aborting the
// dispatching transaction would be bad UX, leave no audit row, and give the batch operator
// undefined retry semantics. Instead record a REJECTED row with a reason and return, so the
// transaction commits and the failure is queryable on Wire (no outbound error attestation).

// (1) the chosen name must be valid for the tier (newnameduser skipped creation otherwise).
if (!valid_name_for_tier(owner, tier)) {
record_nodereg(owner, tier, REJECTED, NAME_INVALID, gen);
return;
}

// (2) newnameduser skips creation at capacity to avoid spending sysio's RAM on a rejected
// claim. Otherwise a missing account means the creation step did not occur.
if (!is_account(owner)) {
const auto reason = nodeowner_count(get_self(), gen, tier) >= nodeowner_cap(tier)
? TIER_CAP_REACHED : OWNER_NOT_ACCOUNT;
record_nodereg(owner, tier, REJECTED, reason, gen);
// Preserve the duplicate outcome for an already registered owner.
nodeowners_t nodeowners(get_self(), gen);
if (nodeowners.contains(nodeowner_key{owner.value})) {
record_nodereg(owner, tier, REJECTED, DUPLICATE, gen);
return;
}

// (3) the account must be controlled by exactly the claimed wire key. For a just-created
// account this is the key newnameduser set; for a pre-existing account it proves the claimant
// controls it -- an NFT cannot be claimed into an account someone else holds.
if (!active_key_matches(owner, wire_pub_key)) {
record_nodereg(owner, tier, REJECTED, ACCOUNT_KEY_MISMATCH, gen);
if (is_account(owner)) {
record_nodereg(owner, tier, REJECTED, ACCOUNT_EXISTS, gen);
return;
}

// (4) must not already be a registered node owner -- a replay / double-claim. Record
// DUPLICATE and return so the claim is idempotent and auditable.
{
nodeowners_t nodeowners(get_self(), gen);
if (nodeowners.contains(nodeowner_key{owner.value})) {
record_nodereg(owner, tier, REJECTED, DUPLICATE, gen);
return;
}
}

// (5) the account must not already carry a *different* EVM link than this claim. recordlink is
// idempotent per (account, chain) and would silently keep the existing key, so a claim with a
// new depositor key would otherwise record CONFIRMED against a stale link. Detect that here and
// soft-fail rather than overwrite or mis-record.
// A trusted import may have recorded a link before account creation. Never confirm
// against a different depositor identity; recordlink intentionally keeps an existing link.
{
authex::links_t links(AUTHEX_ACCOUNT);
auto by_namechain = links.get_index<"bynamechain"_n>();
Expand All @@ -849,22 +825,17 @@ namespace sysio {
return;
}
}

// (6) capacity exhaustion is a terminal claim outcome, not a malformed envelope. Soft-fail
// so another claim in the same consensus envelope cannot roll back every registration and
// leave batch operators retrying the deterministically failing envelope forever.
if (nodeowner_count(get_self(), gen, tier) >= nodeowner_cap(tier)) {
record_nodereg(owner, tier, REJECTED, TIER_CAP_REACHED, gen);
return;
}

// (7) a pre-existing reslimit row no longer blocks registration. An attacker can plant one on
// any account via addpolicy (no target consent), which previously forced this claim into a
// permanent OWNER_HAS_RESLIMIT soft-fail -- a valid registration could be griefed indefinitely.
// regnodeowner now reconciles instead: it stacks the node-owner allocation onto the existing
// row via increase_reslimit rather than creating from scratch. The planted policy stays backed
// by its issuer's reserve and remains independently reclaimable via reducepolicy; no value is
// stolen and the claim reaches CONFIRMED. (SEC-087)
// Queue native newaccount before the privileged allocator. Call the helper directly to
// preserve inline-depth headroom for recordlink's DClaim sweep. All effects, including the

@heifner heifner Nov 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The depth rationale doesn't hold: recordlink is sent from nodeownreg either way, and sending newnameduser as an inline action instead passes all 128 ROA + dispatch cases, including the emitcfg + DClaim case. The real constraint is that allocation runs after the queued newaccount; suggest saying only that.

// terminal audit below, commit together or roll back if an inline action fails.
newnameduser(owner, wire_pub_key, tier);
action(permission_level{get_self(), "active"_n}, get_self(), ROA_FORCEREG,

@heifner heifner Nov 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Every accepted claim now allocates through forcereg. Until now that was a test/bootstrap action (base tester, emissions tests, TestHarness bios, dev clusters) that governance can also call directly through sysio.roa@active. Production traces can no longer tell a governance registration from a claim, and the claim path inherits whatever forcereg becomes.

Suggest a dedicated action that only nodeownreg can reach, same pattern as sysio.epoch::finishadv:

void roa::nodeownfin(const name& owner, const uint8_t& tier) {
   require_auth(get_self());
   check(get_sender() == get_self(), "nodeownfin must be sent inline by sysio.roa");
   check(is_account(owner), "claim account was not created");
   regnodeowner(owner, tier);
}

Send that here, and forcereg stays bootstrap/test only. Keep recordlink sent from nodeownreg: inside the new action it would put linkswept at inline depth 5 and every claim would abort. A test that a top-level push of the new action fails, even signed by sysio.roa@active, pins the get_sender() gate. It adds one action to the ABI, fine pre-launch.

std::make_tuple(owner, tier)).send();

// Record the depositor's ETH key as a sysio.authex link via the trusted depot-only path.
// recordlink requires sysio.authex.active; privileged sysio.roa may declare that permission
Expand All @@ -875,7 +846,6 @@ namespace sysio {
std::make_tuple(owner, opp::types::ChainKind::CHAIN_KIND_EVM, eth_pub_key,
eth_address)).send();

regnodeowner(owner, tier);
record_nodereg(owner, tier, CONFIRMED, NONE, gen);
};

Expand Down Expand Up @@ -911,21 +881,6 @@ namespace sysio {
return len >= 1 && len <= 12;
}

bool roa::active_key_matches(const name& account, const public_key& key) {
// The claim is valid iff `key` can satisfy the account's `active` authority by itself: it
// appears among the keys with weight >= threshold. We deliberately do NOT require a single-key
// authority or an empty `accounts` list -- a standard account also carries an
// <account>@sysio.code entry (benign: it grants the account's own contract inline-action
// rights, not external control), and newnameduser-created accounts have neither. A claim into
// an account a different key controls still fails: that key won't be among `auth.keys`.
auto perm = get_permission(account, "active"_n);
if (!perm.has_value() || perm->auth.threshold == 0) return false;
for (const auto& kw : perm->auth.keys) {
if (kw.key == key && kw.weight >= perm->auth.threshold) return true;
}
return false;
}

uint32_t roa::nodeowner_cap(uint8_t tier) {
switch (tier) {
case 1:
Expand Down Expand Up @@ -1019,7 +974,7 @@ namespace sysio {
// aborting, and never double-counting the gift. Then sync the on-chain quota to the resulting
// row totals so the reslimit row and the resource limits stay equal on BOTH the fresh and
// reconcile paths.
// Conservation: production (newnameduser -> nodeownreg) funds newaccount_ram at creation, so
// Conservation: nodeownreg's newnameduser call funds newaccount_ram before forcereg, so
// the create path stays conservation-exact; the reconcile path adds only the node-owner delta.
// forcereg over a pre-existing row now reconciles too (previously threw in set_reslimit) --
// acceptable for the governance/bootstrap path.
Expand Down Expand Up @@ -1267,17 +1222,15 @@ namespace sysio {
}

void roa::newnameduser(const name& account, const public_key& pubkey, uint8_t tier) {
// Dispatched by the OPP depot (sysio.msgch) in the NFT node-owner claim flow, the same way
// as nodeownreg: privileged sysio.msgch sends this inline declaring {sysio.roa, active},
// without requiring a cross-contract active grant.
// Called directly by nodeownreg, or as a standalone privileged account-creation action.
require_auth(get_self());

roastate_t roastate(get_self());
auto state = roastate.get();
check(state.is_active, "ROA is not active yet");

// Idempotent: if the account already exists, the create step is already done. A key or tier
// mismatch on a pre-existing account is caught downstream by nodeownreg's soft-fail.
// Standalone privileged creation remains idempotent. nodeownreg checks nonexistence
// before calling this helper; a separately created account is not eligible for a claim.
if (is_account(account)) return;

// Tier is a depot/system invariant -- a bad value means the dispatch is malformed, so abort
Expand Down
Loading
Loading