diff --git a/NCC-PLAN.md b/NCC-PLAN.md index afbb9b6..40f36c8 100644 --- a/NCC-PLAN.md +++ b/NCC-PLAN.md @@ -161,6 +161,26 @@ validators are script-invoked pure functions, not implicit dispatch gating). empty tag list. - Route `send ` through the lowering once NIP-17 encrypt-and-gift-wrap lands; it stays deliberately `OperationUnavailable` until then. +- **Top-level and schedule bodies don't execute in source order.** `nscript + run` with no `--event` handles startup two ways, independently: + `top_level_publications` collects every literal-field `publish` outside a + handler/`fn` body and runs it; `top_level_operation_calls` separately + collects every other module operation call outside a handler/`fn` body + and runs *those* through the simulator, reporting each as a flat + `operation N: ` line. Neither honours a `let` binding threading a + value from one into the other — a schedule body computing something + (for example encrypting a payload with `nip44.encrypt_text`) and then + publishing the result is silently split into two disconnected pieces, + and the `print` statements around them never run at all. Every existing + schedule-body example avoided this by only ever publishing literal + fields; `examples/service-operator-bot.ns` hit it building the encrypted + Locator refresh and moved that one sequence into a handler instead + (exactly where NCC-05's own worked example already puts the same + encrypt-then-publish pattern), which is the correct workaround today but + not a fix. A real fix means giving startup execution the same sequential, + data-flow-aware interpretation a handler body gets — an evaluator + change, not a diagnostic — and is deferred rather than attempted + alongside this stage's audit. ## Stages @@ -530,16 +550,22 @@ historical evidence, not latest state. (`handover_role` accepts `predecessor` or `successor`) and one regular event `Handover` (kind 1070, `role`/`handover`/`service`/`p`/`e`/ `effective`/`expires`/`reason`), plus twelve pure functions: `role`, - `handover_id`, `service` and `counterparty` extract a delivered event's - side, transition identifier, service and other identity; + `handover_id`, `handover_service` and `counterparty` extract a delivered + event's side, transition identifier, service and other identity + (`handover_service`, not `service`, since NCC-09 already exports + `service` and importing both modules in one program requires every + declared name across them to be unique — one of four collisions the + whole-library export-uniqueness guard test found on the standard library + built up to this point, see the follow-up below); `proposal_is_valid(tags, created_at)` requires §8.1's shape and refuses a backdated `effective`; `acceptance_is_valid(tags)` requires §9.1's shape; `pair_is_valid(...)` checks §10 across both events — authors, tags, the exact proposal reference (`EventId`), shared identifiers, order and the `expires` window — from values the caller supplies; `effective_time(has_effective, effective, acceptance_created_at)` renders - §11's moment; `state(accepted, effective_at, now)` answers - proposed/accepted/effective; and `conflicts_with(...)`, `continues(...)` + §11's moment; `handover_state(accepted, effective_at, now)` answers + proposed/accepted/effective (named distinctly from NCC-10's `state` for + the same reason); and `conflicts_with(...)`, `continues(...)` and `chain_has_loop(chain)` are §15's ambiguity, §14's link and §14's loop, all as pure data with no clock read inside a host. - **Runtime** dispatch in `nscript-runtime`'s shared pure-function registry @@ -588,7 +614,12 @@ convention defines the container, not what a scope means. `use ncc02`, the `adad778` reference, a `grant_status` validator and the addressable `AuthorityGrant` event (kind 30064: `d`, `service`, `p`, `status`, `scope` list, `expiration`, `valid_from`, `note`), plus ten - pure functions: `operator`/`service`/`status`/`scopes` extract; + pure functions: `operator`/`service`/`authority_status`/`scopes` extract + (`authority_status`, not `status`, since NCC-00 already exports `status` + and importing both modules in one program requires every declared name + across them to be unique — the collision found composing NCC-00 and + NCC-09 while auditing the whole standard library for this class of bug + (see the follow-up below), fixed the same way); `address(service, operator)` builds the `:` `d`; `scope_namespace`/`scope_is_valid` classify §8's `ncc:`/`pubkey:` forms and leave everything else opaque; `grant_starts_at(tags, created_at)` @@ -765,6 +796,49 @@ scope rather than a general SemVer range library. Dependency resolution (§14) and installation itself are explicitly out of NCC-13's scope and are not implemented here. +## Cross-module export audit (after stage 11) + +Every stage so far reviewed one convention against the pin and, where its +own author noticed, against the modules it explicitly composes with (stage +9's `operator_service` naming note is one such case). Nothing checked a +new module's declared names against the *entire* standard library, because +`nscript-semantics`'s `E4003` ambiguous-export check only ever runs per +program, over whichever modules that program's own `use` graph pulls in — +two modules can each check and test cleanly on their own and still collide +the moment a script imports both. + +Building `examples/service-operator-bot.ns` (composing NCC-02/05/06/07/09/ +10/11/13 in one program) hit exactly this: NCC-13 had reused NCC-10's +`operator_for_tag`/`operator_project`/`operator_application` verbatim. +Fixing it and then auditing the whole standard library (a new +`nscript-modules` test, `built_in_modules_export_no_ambiguous_names_ +across_the_standard_library`, replicating `E4003`'s name-collection logic +over every `BUILTINS` entry at once rather than one program's import +graph) found three more, none previously exercised by any program that +imports both sides: + +- `service`: NCC-08 (a handover's service) and NCC-09 (a grant's service). + NCC-08's renamed to `handover_service`. +- `status`: NCC-00 (a document's lifecycle status) and NCC-09 (a grant's + status). NCC-09's renamed to `authority_status`. +- `state`: NCC-08 (a handover's proposed/accepted/effective judgment, + `state(accepted, effective_at, now)`) and NCC-10 (a service's tag, + `state(tags)`) — coincidentally same name, unrelated signatures and + purposes. NCC-08's renamed to `handover_state`. + +Each rename picked whichever side had fewer existing call sites to update, +matching the precedent NCC-13's own fix set (nothing outside the renamed +module's own fixtures depended on the old name). The audit also found two +long-standing collisions outside the NCC family — `concord02` and `nip72` +both declared `CommunityError`, and `concord02` and `nip28` both declared +`Channel` — renamed to `ConcordCommunityError`/`ConcordChannel` (concord02 +had zero external call sites for either, so the rename touched only its +own module file). + +The guard test now runs on every `cargo test`, so a future module reusing +an already-shipped name fails the build immediately rather than waiting +for some later script to compose the two and discover it by hand. + ## Guardrails - No NCC-specific syntax: conventions arrive as typed modules with records, diff --git a/conformance/valid/ncc08-handover.ns b/conformance/valid/ncc08-handover.ns index f1355bb..46cf846 100644 --- a/conformance/valid/ncc08-handover.ns +++ b/conformance/valid/ncc08-handover.ns @@ -46,8 +46,8 @@ on Handover where tags.role contains "predecessor" { if not ncc08.proposal_is_valid(event.tags, event.created_at) { print("proposal rejected: incomplete or backdated") } else { - print("proposal " + ncc08.handover_id(event.tags) + " moves service " + ncc08.service(event.tags) + " to " + ncc08.counterparty(event.tags)) - print("a proposal alone is " + ncc08.state(false, 0, event.created_at) + ", never accepted") + print("proposal " + ncc08.handover_id(event.tags) + " moves service " + ncc08.handover_service(event.tags) + " to " + ncc08.counterparty(event.tags)) + print("a proposal alone is " + ncc08.handover_state(false, 0, event.created_at) + ", never accepted") } } @@ -55,7 +55,7 @@ on Handover where tags.role contains "successor" { if not ncc08.acceptance_is_valid(event.tags) { print("acceptance rejected: incomplete") } else { - print("acceptance " + ncc08.handover_id(event.tags) + " acknowledges " + ncc08.counterparty(event.tags) + " for service " + ncc08.service(event.tags)) + print("acceptance " + ncc08.handover_id(event.tags) + " acknowledges " + ncc08.counterparty(event.tags) + " for service " + ncc08.handover_service(event.tags)) print("with no effective tag the pair is effective at " + text(ncc08.effective_time(false, 0, event.created_at))) } } diff --git a/crates/nscript-modules/src/resolver.rs b/crates/nscript-modules/src/resolver.rs index 2223d52..00f10c7 100644 --- a/crates/nscript-modules/src/resolver.rs +++ b/crates/nscript-modules/src/resolver.rs @@ -738,7 +738,7 @@ fn dependency_cycle(selected: &BTreeMap) -> Option ModuleDescriptor { @@ -865,6 +865,55 @@ mod tests { assert_eq!(registry.load_root(&root).unwrap(), 93); } + /// Every built-in module's declared name (a type, event, tag, error, + /// function or operation) must be unique across the *entire* standard + /// library, not just within the modules any one program happens to + /// import together: `nscript-semantics`'s `E4003` ambiguous-export check + /// runs per program, over whichever modules its own `use` graph pulls + /// in, so two builtins can each check and test cleanly on their own and + /// still collide the moment a script imports both — exactly what + /// happened when NCC-13 (reviewed and shipped independently) reused + /// NCC-10's `operator_for_tag`/`operator_project`/`operator_application` + /// verbatim, caught only by hand while composing both into + /// `examples/service-operator-bot.ns`. This test makes that class of + /// collision a build failure instead of something a future NCC or NIP + /// module stage has to discover by accident. + #[test] + fn built_in_modules_export_no_ambiguous_names_across_the_standard_library() { + let mut owners: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + let mut collisions = Vec::new(); + for &(module_name, source) in BUILTINS { + let (descriptor, diagnostics) = crate::parse_module(source); + let descriptor = descriptor + .unwrap_or_else(|| panic!("invalid built-in {module_name}: {diagnostics:#?}")); + let names = descriptor + .types + .iter() + .map(crate::TypeDefinition::name) + .chain(descriptor.events.iter().map(|item| item.name.as_str())) + .chain(descriptor.tags.iter().map(|item| item.name.as_str())) + .chain(descriptor.errors.iter().map(|item| item.name.as_str())) + .chain(descriptor.functions.iter().map(|item| item.name.as_str())) + .chain(descriptor.operations.iter().map(|item| item.name.as_str())); + for name in names { + match owners.insert(name.to_owned(), module_name) { + Some(first) if first != module_name => { + collisions.push(format!( + "`{name}` is provided by `{first}` and `{module_name}`" + )); + } + _ => {} + } + } + } + assert!( + collisions.is_empty(), + "ambiguous exports across the standard library:\n{}", + collisions.join("\n") + ); + } + #[test] fn resolves_private_message_module_graph() { let registry = ModuleRegistry::with_builtins(); diff --git a/crates/nscript-runtime/src/lib.rs b/crates/nscript-runtime/src/lib.rs index ce01936..8c4a635 100644 --- a/crates/nscript-runtime/src/lib.rs +++ b/crates/nscript-runtime/src/lib.rs @@ -3571,19 +3571,19 @@ fn pure_function(module: &str, function: &str) -> Option { ("ncc07", "capability_is_valid") => Some(ncc07_capability_is_valid as PureFunction), ("ncc08", "role") => Some(ncc08_role as PureFunction), ("ncc08", "handover_id") => Some(ncc08_handover_id as PureFunction), - ("ncc08", "service") => Some(ncc08_service as PureFunction), + ("ncc08", "handover_service") => Some(ncc08_handover_service as PureFunction), ("ncc08", "counterparty") => Some(ncc08_counterparty as PureFunction), ("ncc08", "proposal_is_valid") => Some(ncc08_proposal_is_valid as PureFunction), ("ncc08", "acceptance_is_valid") => Some(ncc08_acceptance_is_valid as PureFunction), ("ncc08", "pair_is_valid") => Some(ncc08_pair_is_valid as PureFunction), ("ncc08", "effective_time") => Some(ncc08_effective_time as PureFunction), - ("ncc08", "state") => Some(ncc08_state as PureFunction), + ("ncc08", "handover_state") => Some(ncc08_handover_state as PureFunction), ("ncc08", "conflicts_with") => Some(ncc08_conflicts_with as PureFunction), ("ncc08", "continues") => Some(ncc08_continues as PureFunction), ("ncc08", "chain_has_loop") => Some(ncc08_chain_has_loop as PureFunction), ("ncc09", "operator") => Some(ncc09_operator as PureFunction), ("ncc09", "service") => Some(ncc09_service as PureFunction), - ("ncc09", "status") => Some(ncc09_status as PureFunction), + ("ncc09", "authority_status") => Some(ncc09_authority_status as PureFunction), ("ncc09", "scopes") => Some(ncc09_scopes as PureFunction), ("ncc09", "address") => Some(ncc09_address as PureFunction), ("ncc09", "scope_namespace") => Some(ncc09_scope_namespace as PureFunction), @@ -5122,8 +5122,8 @@ fn ncc08_handover_id(arguments: &[OperationValue]) -> Result Result { - tag_value(arguments, "service", "ncc08.service") +fn ncc08_handover_service(arguments: &[OperationValue]) -> Result { + tag_value(arguments, "service", "ncc08.handover_service") } /// NCC-08 §8.1/§9.1: the other side of the handover named by the `p` @@ -5294,14 +5294,14 @@ fn ncc08_effective_time(arguments: &[OperationValue]) -> Result Result { +fn ncc08_handover_state(arguments: &[OperationValue]) -> Result { let [ OperationValue::Bool(accepted), OperationValue::Integer(effective_at), OperationValue::Integer(now), ] = arguments else { - return Err(invalid("ncc08.state")); + return Err(invalid("ncc08.handover_state")); }; let state = if !accepted { "proposed" @@ -5398,9 +5398,12 @@ fn ncc09_service(arguments: &[OperationValue]) -> Result Result { - tag_value(arguments, "status", "ncc09.status") +/// NCC-09 §6.3: a grant's status, `active` or `revoked`. Named distinctly +/// from NCC-00's `status` (a document's lifecycle status) because +/// importing both modules in one program requires every declared name +/// across them to be unique. +fn ncc09_authority_status(arguments: &[OperationValue]) -> Result { + tag_value(arguments, "status", "ncc09.authority_status") } /// NCC-09 §6.3: every scope the grant lists, in order, skipping empty ones. @@ -11016,7 +11019,7 @@ mod tests { text(&handover) ); assert_eq!( - host.call_pure_function("ncc08", "service", &[proposal()]) + host.call_pure_function("ncc08", "handover_service", &[proposal()]) .unwrap(), text("relay") ); @@ -11313,7 +11316,7 @@ mod tests { let mut state = |accepted: bool, effective_at: i64, now: i64| { host.call_pure_function( "ncc08", - "state", + "handover_state", &[ OperationValue::Bool(accepted), OperationValue::Integer(effective_at), @@ -11458,7 +11461,7 @@ mod tests { }; assert_eq!(extract("operator"), text(&operator)); assert_eq!(extract("service"), text("relay")); - assert_eq!(extract("status"), text("active")); + assert_eq!(extract("authority_status"), text("active")); assert_eq!( extract("scopes"), OperationValue::List(vec![ diff --git a/examples/ncc08-rotation-bot.ns b/examples/ncc08-rotation-bot.ns index 5b11d80..105714a 100644 --- a/examples/ncc08-rotation-bot.ns +++ b/examples/ncc08-rotation-bot.ns @@ -51,15 +51,15 @@ permissions { on Handover where tags.role contains "predecessor" { if not ncc08.proposal_is_valid(event.tags, event.created_at) { print("proposal rejected: incomplete or backdated") - } else if ncc08.service(event.tags) != "relay" { - print("proposal ignored: this bot takes over `relay` only, not " + ncc08.service(event.tags)) + } else if ncc08.handover_service(event.tags) != "relay" { + print("proposal ignored: this bot takes over `relay` only, not " + ncc08.handover_service(event.tags)) } else { // The predecessor is the proposal's own author — a real // pubkey — not the successor the `p` tag addresses, and not // the reason text any proposer could write. let predecessor: PubKey = event.author let handover = ncc08.handover_id(event.tags) - let service = ncc08.service(event.tags) + let service = ncc08.handover_service(event.tags) let published = publish Handover { role: "successor"; handover: handover; @@ -76,6 +76,6 @@ on Handover where tags.role contains "successor" { if not ncc08.acceptance_is_valid(event.tags) { print("acceptance rejected: incomplete") } else { - print("acceptance " + ncc08.handover_id(event.tags) + " acknowledges " + ncc08.counterparty(event.tags) + " for " + ncc08.service(event.tags)) + print("acceptance " + ncc08.handover_id(event.tags) + " acknowledges " + ncc08.counterparty(event.tags) + " for " + ncc08.handover_service(event.tags)) } } diff --git a/modules/std/concord02/0.1.0.nsm b/modules/std/concord02/0.1.0.nsm index 8bd1345..b031a73 100644 --- a/modules/std/concord02/0.1.0.nsm +++ b/modules/std/concord02/0.1.0.nsm @@ -15,12 +15,12 @@ nominal Epoch from Integer { require value >= 0 } -record Channel { +record ConcordChannel { field id: ChannelId field name: Text field private: Bool } -error CommunityError { +error ConcordCommunityError { field message: Text } diff --git a/modules/std/ncc08/0.1.0.nsm b/modules/std/ncc08/0.1.0.nsm index a4d459a..afba130 100644 --- a/modules/std/ncc08/0.1.0.nsm +++ b/modules/std/ncc08/0.1.0.nsm @@ -9,13 +9,13 @@ validator handover_role(value: Text) { function role(tags: List) -> Text function handover_id(tags: List) -> Text -function service(tags: List) -> Text +function handover_service(tags: List) -> Text function counterparty(tags: List) -> Text function proposal_is_valid(tags: List, created_at: Int) -> Bool function acceptance_is_valid(tags: List) -> Bool function pair_is_valid(proposal_author: PubKey, proposal_tags: List, proposal_id: EventId, proposal_created_at: Int, acceptance_author: PubKey, acceptance_tags: List, acceptance_created_at: Int) -> Bool function effective_time(has_effective: Bool, effective: Int, acceptance_created_at: Int) -> Int -function state(accepted: Bool, effective_at: Int, now: Int) -> Text +function handover_state(accepted: Bool, effective_at: Int, now: Int) -> Text function conflicts_with(service: Text, predecessor: Text, successor: Text, other_service: Text, other_predecessor: Text, other_successor: Text) -> Bool function continues(service: Text, successor: Text, next_service: Text, next_predecessor: Text) -> Bool function chain_has_loop(chain: List) -> Bool diff --git a/modules/std/ncc09/0.1.0.nsm b/modules/std/ncc09/0.1.0.nsm index 3e84f50..5f3fd18 100644 --- a/modules/std/ncc09/0.1.0.nsm +++ b/modules/std/ncc09/0.1.0.nsm @@ -9,7 +9,7 @@ validator grant_status(value: Text) { function operator(tags: List) -> Text function service(tags: List) -> Text -function status(tags: List) -> Text +function authority_status(tags: List) -> Text function scopes(tags: List) -> List function address(service: Text, operator: Text) -> Text function scope_namespace(scope: Text) -> Text diff --git a/spec/nostr.md b/spec/nostr.md index 51b8368..b8fd9a6 100644 --- a/spec/nostr.md +++ b/spec/nostr.md @@ -353,7 +353,7 @@ addressable ones, both stay historical evidence instead of being replaced by the author's next revision. Validation is exposed as pure functions. `ncc08.role(event.tags)`, -`ncc08.handover_id(event.tags)`, `ncc08.service(event.tags)` and +`ncc08.handover_id(event.tags)`, `ncc08.handover_service(event.tags)` and `ncc08.counterparty(event.tags)` extract the side, the transition identifier, the service, and the other identity the `role` addresses; `ncc08.proposal_is_valid(event.tags, created_at)` requires §8.1's shape and @@ -368,7 +368,7 @@ Signatures remain the host's business, never the module's. Time and state are pure too. `ncc08.effective_time(has_effective, effective, acceptance_created_at)` renders §11's effective moment — the proposal's `effective` when it carried one, else the acceptance's own -time — and `ncc08.state(accepted, effective_at, now)` answers `proposed`, +time — and `ncc08.handover_state(accepted, effective_at, now)` answers `proposed`, `accepted` or `effective`: a proposal alone is never accepted, and an accepted pair becomes effective only once `now` reaches that moment, with the predecessor still current until then. Chain and conflict rules are @@ -404,8 +404,11 @@ its scopes are never inherited (§14), while a grant revoked with The operator never becomes the service (§4) and the convention defines the authority container, not what a scope means (§9). Extraction stays pure: -`ncc09.operator`, `ncc09.service`, `ncc09.status` and `ncc09.scopes` read -the grant, and `ncc09.scope_namespace`/`ncc09.scope_is_valid` report the +`ncc09.operator`, `ncc09.service`, `ncc09.authority_status` and +`ncc09.scopes` read the grant (`authority_status`, not `status`, because +NCC-00 already exports `status` and importing both modules in one program +requires every declared name across them to be unique), and +`ncc09.scope_namespace`/`ncc09.scope_is_valid` report the `ncc::` and `pubkey::` spellings (everything else is opaque). `ncc09.grant_starts_at(tags, created_at)` renders §6.4's start — `valid_from` when present, else the event's own time — and