diff --git a/CHANGELOG.md b/CHANGELOG.md index d1eba19eab..674813a3d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ The main work (all changes without a GitHub username in brackets in the below li ## __WORK IN PROGRESS__ --> +## __WORK IN PROGRESS__ + +- @matter/protocol + - Fix: The peer's medium-specific MRP retransmission margin now applies to peer-initiated exchanges (e.g. subscription data reports) and to exchanges created without peer context + - Enhancement: Network profiles carry a separate `bdxAdditionalMrpDelay` for bulk transfer; thread applies no additional margin to BDX + - Fix: Grouped `PeerTimingParameters` (`kickRestartCooldown`, `addressChangeProbeCooldown`) merge field-wise, so overriding one member keeps its siblings + +- @matter/node + - Enhancement: `network.profiles` accepts `bdxAdditionalMrpDelay`, and `network.timing` accepts the kick and address-change parameters + ## 0.17.7 (2026-07-27) - @matter/general diff --git a/packages/node/src/behavior/system/network/NetworkServer.ts b/packages/node/src/behavior/system/network/NetworkServer.ts index a782ff300c..1fc0a0cca3 100644 --- a/packages/node/src/behavior/system/network/NetworkServer.ts +++ b/packages/node/src/behavior/system/network/NetworkServer.ts @@ -5,7 +5,7 @@ */ import { ServerSubscriptionConfig } from "#node/server/ServerSubscription.js"; -import { Duration, Logger, Minutes } from "@matter/general"; +import { DeepPartial, Duration, Logger, Minutes } from "@matter/general"; import { duration, field, uint16 } from "@matter/model"; import { Ble, FabricManager, NetworkProfiles, PeerTimingParameters } from "@matter/protocol"; import { DiscoveryCapabilitiesBitmap, TypeFromPartialBitSchema } from "@matter/types"; @@ -78,7 +78,25 @@ export namespace NetworkServer { declare runtime: ServerNetworkRuntime; } - export class TimingConfig implements Partial { + export class KickRestartCooldownConfig implements Partial { + @field(duration) + addressChange?: Duration; + + @field(duration) + connect?: Duration; + } + + export class AddressChangeProbeCooldownConfig implements Partial< + PeerTimingParameters["addressChangeProbeCooldown"] + > { + @field(duration) + minimum?: Duration; + + @field(duration) + maximum?: Duration; + } + + export class TimingConfig implements DeepPartial { @field(duration) defaultConnectionTimeout?: Duration; @@ -98,7 +116,22 @@ export namespace NetworkServer { delayAfterUnhandledError?: Duration; @field(duration) - minimumTimeBetweenMrpKicks?: Duration; + kickThrottleInterval?: Duration; + + @field(uint16) + kickMinRetransmissions?: number; + + @field(duration) + kickMinRestartSaving?: Duration; + + @field(KickRestartCooldownConfig) + kickRestartCooldown?: KickRestartCooldownConfig; + + @field(duration) + addressChangeStabilizationDelay?: Duration; + + @field(AddressChangeProbeCooldownConfig) + addressChangeProbeCooldown?: AddressChangeProbeCooldownConfig; } export class ConcreteLimitsConfig implements Partial { @@ -113,6 +146,9 @@ export namespace NetworkServer { @field(duration) additionalMrpDelay?: Duration; + + @field(duration) + bdxAdditionalMrpDelay?: Duration; } export class LimitsConfig extends ConcreteLimitsConfig { @@ -146,6 +182,23 @@ export namespace NetworkServer { icdLit?: LimitsConfig; } + type Mismatched = Exclude | Exclude; + type SameFields<_Mismatched extends never> = unknown; + + /** + * The config classes above restate their source's fields because {@link field} needs real property declarations, + * and `implements Partial` catches neither a field they fail to declare nor one the source has dropped. + * This does: either divergence breaks the build here, naming the offending field. + */ + export type ConfigMatchesItsSource = [ + SameFields>, + SameFields>, + SameFields>, + SameFields>, + SameFields>, + SameFields>, + ]; + export class State extends NetworkBehavior.State { listeningAddressIpv4?: string = undefined; listeningAddressIpv6?: string = undefined; diff --git a/packages/node/test/node/ClientTuningTest.ts b/packages/node/test/node/ClientTuningTest.ts index 5bfc93560b..5376889481 100644 --- a/packages/node/test/node/ClientTuningTest.ts +++ b/packages/node/test/node/ClientTuningTest.ts @@ -12,7 +12,14 @@ import { Endpoint } from "#endpoint/Endpoint.js"; import { SecondaryNetworkInterfaceEndpoint } from "#endpoints/secondary-network-interface"; import { ServerNode } from "#index.js"; import { Bytes, causedBy, Crypto, Millis, MockCrypto, MockNetwork, Network, Seconds } from "@matter/general"; -import { NetworkProfiles, PeerSet, PeerTimingParameters, PeerUnreachableError, SessionManager } from "@matter/protocol"; +import { + MdnsService, + NetworkProfiles, + PeerSet, + PeerTimingParameters, + PeerUnreachableError, + SessionManager, +} from "@matter/protocol"; import { NetworkCommissioning } from "@matter/types/clusters/network-commissioning"; import { ThreadNetworkDiagnostics } from "@matter/types/clusters/thread-network-diagnostics"; import { MockServerNode } from "./mock-server-node.js"; @@ -73,6 +80,37 @@ describe("ClientTuningTest", () => { expect(peer.network.additionalMrpDelay).equals(Seconds(1)); }); + it("exposes the peer's MRP margin on the session so peer-initiated exchanges inherit it", async () => { + await using site = new MockSite(); + const controller = await site.addController(); + const device = await site.addNode(WifiRoot, { device: OnOffLightDevice }); + + await commission(controller, device); + + const peer = [...controller.env.get(PeerSet)][0]; + expect(peer.newestSession()?.peerMrpMargins?.messaging).equals(Seconds(1)); + }); + + it("adopts sessions that predate its construction", async () => { + await using site = new MockSite(); + const controller = await site.addController(); + const device = await site.addNode(WifiRoot, { device: OnOffLightDevice }); + + await commission(controller, device); + + const { env } = controller; + const late = new PeerSet({ + lifetime: env, + sessions: env.get(SessionManager), + names: env.get(MdnsService).names, + networks: env.get(NetworkProfiles), + }); + + const peer = [...late][0]; + expect(peer?.address).deep.equals([...env.get(PeerSet)][0].address); + expect(peer.sessions.size).equals(1); + }); + it("uses default timing when not configured", async () => { await using site = new MockSite(); const { controller } = await site.addCommissionedPair(); @@ -142,6 +180,33 @@ describe("ClientTuningTest", () => { expect(conservative.additionalMrpDelay).equals(Seconds(1.5)); }); + it("a configured bdxAdditionalMrpDelay propagates to NetworkProfiles", async () => { + await using site = new MockSite(); + + const controller = await site.addController({ + network: { + profiles: { + thread: { bdxAdditionalMrpDelay: Seconds(8) }, + wifi: { bdxAdditionalMrpDelay: Seconds(2) }, + }, + }, + }); + const device = await site.addDevice(); + await commission(controller, device); + + const profiles = controller.env.get(NetworkProfiles); + + const thread = profiles.get("thread"); + expect(thread.bdxAdditionalMrpDelay).equals(Seconds(8)); + expect(thread.additionalMrpDelay).equals(Seconds(1.5)); + + const wifi = profiles.get("wifi"); + expect(wifi.bdxAdditionalMrpDelay).equals(Seconds(2)); + expect(wifi.additionalMrpDelay).equals(Seconds(1)); + + expect(profiles.get("conservative").bdxAdditionalMrpDelay).equals(Seconds(1.5)); + }); + it("ownNetworkProfileId sets the sender-side MRP margin", async () => { await using site = new MockSite(); diff --git a/packages/protocol/src/peer/NetworkProfile.ts b/packages/protocol/src/peer/NetworkProfile.ts index 87f2c9fc6c..800805562b 100644 --- a/packages/protocol/src/peer/NetworkProfile.ts +++ b/packages/protocol/src/peer/NetworkProfile.ts @@ -5,6 +5,7 @@ */ import { PeerAddress } from "#peer/PeerAddress.js"; +import { MRP } from "#protocol/MRP.js"; import { Diagnostic, Duration, @@ -44,6 +45,13 @@ export interface ConcreteNetworkProfile { * with the local "own" profile margin via max at send time. */ additionalMrpDelay: Duration; + + /** + * {@link additionalMrpDelay} for bulk transfer (BDX) exchanges, which sustain many round trips over one path. + * + * Defaults to {@link additionalMrpDelay} so a profile that does not distinguish traffic classes behaves as before. + */ + bdxAdditionalMrpDelay: Duration; } /** @@ -138,25 +146,28 @@ export class NetworkProfiles { return this.configure(id, this.#defaults[id as keyof NetworkProfiles.Templates]); } - configure(id: string, limits: NetworkProfiles.Limits, parentDelay?: Duration) { - const additionalMrpDelay = limits.additionalMrpDelay ?? parentDelay ?? Millis(0); + configure(id: string, limits: NetworkProfiles.Limits, parent?: MRP.Margins) { + const additionalMrpDelay = limits.additionalMrpDelay ?? parent?.messaging ?? Millis(0); + const bdxAdditionalMrpDelay = limits.bdxAdditionalMrpDelay ?? parent?.bdx ?? additionalMrpDelay; const network: NetworkProfile = { id, semaphore: new Semaphore(`network semaphore ${id}`, limits.exchanges, limits.delay, limits.timeout), additionalMrpDelay, + bdxAdditionalMrpDelay, }; + const inherited = { messaging: additionalMrpDelay, bdx: bdxAdditionalMrpDelay }; if (limits.connect) { network.connect = this.configure( `${id}:connect`, { ...limits.connect, connect: undefined, probeAddress: undefined }, - additionalMrpDelay, + inherited, ); } if (limits.probeAddress) { network.probeAddress = this.configure( `${id}:probe`, { ...limits.probeAddress, connect: undefined, probeAddress: undefined }, - additionalMrpDelay, + inherited, ); } logger.info( @@ -241,6 +252,11 @@ export namespace NetworkProfiles { * Additive MRP retransmission margin for this medium. Defaults to 0 unless the template sets one. */ additionalMrpDelay?: Duration; + + /** + * {@link additionalMrpDelay} for bulk transfer (BDX) exchanges. Defaults to {@link additionalMrpDelay}. + */ + bdxAdditionalMrpDelay?: Duration; } /** @@ -344,7 +360,8 @@ export namespace NetworkProfiles { icdLit: { exchanges: Infinity, additionalMrpDelay: Millis(0) }, fast: { exchanges: 200, additionalMrpDelay: Millis(0) }, wifi: { exchanges: 200, additionalMrpDelay: Seconds(1) }, - thread: conservative, + // The explicit zero matters: omitting it would inherit conservative's messaging margin for BDX too + thread: { ...conservative, bdxAdditionalMrpDelay: Seconds(0) }, conservative, unknown: conservative, }; diff --git a/packages/protocol/src/peer/Peer.ts b/packages/protocol/src/peer/Peer.ts index b7ee9113c3..8876347724 100644 --- a/packages/protocol/src/peer/Peer.ts +++ b/packages/protocol/src/peer/Peer.ts @@ -166,6 +166,16 @@ export class Peer { // Ensure session parameters reflect those most recently reported by peer this.#descriptor.sessionParameters = session.parameters; + + // Only pad when we actually know the peer's medium; the "unknown" fallback is deliberately not applied + // here because it would inflate responder timing for every peer of a node that never characterizes them. + session.peerMrpMarginsResolver = () => { + if (this.#physicalProperties === undefined) { + return undefined; + } + const { additionalMrpDelay, bdxAdditionalMrpDelay } = this.network; + return { messaging: additionalMrpDelay, bdx: bdxAdditionalMrpDelay }; + }; }); } diff --git a/packages/protocol/src/peer/PeerSet.ts b/packages/protocol/src/peer/PeerSet.ts index 91f66d4ee8..8b94158d18 100644 --- a/packages/protocol/src/peer/PeerSet.ts +++ b/packages/protocol/src/peer/PeerSet.ts @@ -7,6 +7,7 @@ import { MdnsService } from "#mdns/MdnsService.js"; import { PeerAddress } from "#peer/PeerAddress.js"; import { ExchangeManager } from "#protocol/ExchangeManager.js"; +import type { NodeSession } from "#session/NodeSession.js"; import { Session } from "#session/Session.js"; import { SessionManager } from "#session/SessionManager.js"; import { @@ -14,6 +15,7 @@ import { AsyncObservable, BasicSet, ChannelType, + DeepPartial, DnssdNames, Environment, Environmental, @@ -29,7 +31,6 @@ import { RetrySchedule, ServerAddressIp, } from "@matter/general"; -import { FabricIndex } from "@matter/types"; import { NetworkProfiles } from "./NetworkProfile.js"; import { Peer } from "./Peer.js"; import { PeerConnection } from "./PeerConnection.js"; @@ -91,7 +92,8 @@ export class PeerSet implements ImmutableSet, ObservableSet { join: name => this.#lifetime.join(name), }; - this.#peers.added.on(peer => { + this.#observers.on(this.#peers.added, peer => { + // Scoped to the peer's own observable, so it needs no cleanup of ours peer.sessions.deleted.on(() => { if (!peer.sessions.size) { this.#disconnected.emit(peer); @@ -99,24 +101,27 @@ export class PeerSet implements ImmutableSet, ObservableSet { }); }); - this.#sessions.sessions.added.on(session => { - if (session.peerAddress.fabricIndex === FabricIndex.NO_FABRIC || session.isClosed) { - return; - } + this.#observers.on(this.#sessions.sessions.added, session => this.#adoptSession(session)); - this.addKnownPeer({ - address: session.peerAddress, - operationalAddress: operationalAddressOf(session), - }); - }); + // Sessions may predate us: nothing guarantees we are instantiated before the node establishes its first + // operational session + for (const session of this.#sessions.sessions) { + this.#adoptSession(session); + } + } - this.#observers.on(this.#sessions.sessions.added, session => { - if (session.fabric === undefined) { - return; - } + /** + * Associate an operational session with its peer, creating the peer if we do not know it yet. + */ + #adoptSession(session: NodeSession) { + if (session.fabric === undefined || session.isClosed) { + return; + } - this.for(session.peerAddress).sessions.add(session); - }); + this.addKnownPeer({ + address: session.peerAddress, + operationalAddress: operationalAddressOf(session), + }).sessions.add(session); } set exchanges(exchanges: ExchangeManager | undefined) { @@ -222,7 +227,7 @@ export class PeerSet implements ImmutableSet, ObservableSet { return this.#peerContext.timing; } - set timing(timing: Partial) { + set timing(timing: DeepPartial) { this.#peerContext.timing = PeerTimingParameters(timing); } diff --git a/packages/protocol/src/peer/PeerTimingParameters.ts b/packages/protocol/src/peer/PeerTimingParameters.ts index 05288a2bf7..39a1364488 100644 --- a/packages/protocol/src/peer/PeerTimingParameters.ts +++ b/packages/protocol/src/peer/PeerTimingParameters.ts @@ -4,7 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Duration, Hours, merge as mergeObjects, Millis, Minutes, Seconds } from "@matter/general"; +import { + DeepPartial, + Duration, + Hours, + isObject, + merge as mergeObjects, + Millis, + Minutes, + Seconds, +} from "@matter/general"; /** * Parameters that control network timing for Matter sessions controlled by matter.js. @@ -129,18 +138,28 @@ interface Internal extends PeerTimingParameters { [complete]: true; } -export function PeerTimingParameters(options?: Partial) { +export function PeerTimingParameters(options?: DeepPartial) { if (options && (options as Internal)[complete]) { return options as PeerTimingParameters; } - const result = { ...PeerTimingParameters.defaults } as Record; - if (options) { - for (const key of Object.keys(options)) { - const value = (options as Record)[key]; - if (value !== undefined) { - result[key] = value; + return applyOverrides(PeerTimingParameters.defaults, options); +} + +/** + * Layer overrides onto a complete parameter set. Grouped parameters merge field-wise, so overriding one member of a + * group leaves its siblings at their prior value rather than dropping them. + */ +function applyOverrides(base: PeerTimingParameters, overrides?: DeepPartial) { + const result = { ...base } as Record; + if (overrides) { + for (const key of Object.keys(overrides)) { + const value = (overrides as Record)[key]; + if (value === undefined) { + continue; } + const current = result[key]; + result[key] = isObject(current) && isObject(value) ? mergeObjects(current, value) : value; } } result[complete] = true; @@ -155,10 +174,11 @@ export namespace PeerTimingParameters { * Unlike calling {@link PeerTimingParameters} directly (which merges on top of {@link defaults}), this starts from * an arbitrary base. Only override fields that are explicitly defined and non-undefined are applied. */ - export function merge(base: PeerTimingParameters, overrides: Partial): PeerTimingParameters { - const result = mergeObjects(base, overrides) as Internal; - result[complete] = true; - return result; + export function merge( + base: PeerTimingParameters, + overrides: DeepPartial, + ): PeerTimingParameters { + return applyOverrides(base, overrides); } const maxInitialContactRetryInterval = Minutes(2); diff --git a/packages/protocol/src/protocol/ExchangeProvider.ts b/packages/protocol/src/protocol/ExchangeProvider.ts index 75356be7d4..8f14ea5f49 100644 --- a/packages/protocol/src/protocol/ExchangeProvider.ts +++ b/packages/protocol/src/protocol/ExchangeProvider.ts @@ -130,8 +130,6 @@ export class DedicatedChannelExchangeProvider extends ExchangeProvider { } async initiateExchange(options?: NewExchangeOptions): Promise { - // This provider has no peer/medium context, so the medium-derived margin is unavailable; only an explicit - // per-call override can apply here. return this.exchangeManager.initiateExchangeForSession(this.#session, INTERACTION_PROTOCOL_ID, { peerAdditionalMrpDelay: options?.additionalMrpDelay, }); diff --git a/packages/protocol/src/protocol/MRP.ts b/packages/protocol/src/protocol/MRP.ts index 76ac29d21b..ae3bdcf79e 100644 --- a/packages/protocol/src/protocol/MRP.ts +++ b/packages/protocol/src/protocol/MRP.ts @@ -6,8 +6,26 @@ import { SessionParameters } from "#session/SessionParameters.js"; import { ChannelType, Duration, MatterFlowError, Millis, Seconds } from "@matter/general"; +import { BDX_PROTOCOL_ID } from "@matter/types"; export namespace MRP { + /** + * Additive retransmission margins for a network, by traffic class. Bulk transfer sustains many round trips over + * the same path, so it tolerates — and on a constrained medium needs — a different margin than normal messaging. + */ + export interface Margins { + messaging: Duration; + bdx: Duration; + } + + /** Selects the margin a protocol's traffic belongs to. */ + export function marginFor(margins: Margins | undefined, protocolId: number) { + if (margins === undefined) { + return undefined; + } + return protocolId === BDX_PROTOCOL_ID ? margins.bdx : margins.messaging; + } + /** * The maximum number of transmission attempts for a given reliable message. The sender MAY choose this value as it * sees fit. diff --git a/packages/protocol/src/protocol/MessageExchange.ts b/packages/protocol/src/protocol/MessageExchange.ts index 9f681c26b2..ae52dd85c8 100644 --- a/packages/protocol/src/protocol/MessageExchange.ts +++ b/packages/protocol/src/protocol/MessageExchange.ts @@ -1044,9 +1044,13 @@ export class MessageExchange { return this.#backOffFor(this.#retransmissionCounter); } - /** Amplified backoff addition applied to our sends: the larger of our own and the peer's network-profile delay. */ + /** + * Amplified backoff addition applied to our sends: the larger of our own and the peer's network-profile delay for + * this exchange's traffic class. + */ get #sendAdditionalDelay() { - return Duration.max(this.#context.localAdditionalMrpDelay, this.#peerAdditionalMrpDelay ?? Millis(0)); + const peerMargin = this.#peerAdditionalMrpDelay ?? MRP.marginFor(this.session.peerMrpMargins, this.#protocolId); + return Duration.max(this.#context.localAdditionalMrpDelay, peerMargin ?? Millis(0)); } /** @@ -1111,6 +1115,9 @@ export namespace MessageExchange { /** * Additive MRP retransmission margin for the peer's network medium. Sourced independently of * {@link network} so concurrency overrides cannot strip the medium-correct margin (e.g. thread's). + * + * Overrides {@link Session.peerMrpMargins}, which supplies the margin for exchanges created without this + * option (notably peer-initiated ones). */ peerAdditionalMrpDelay?: Duration; diff --git a/packages/protocol/src/session/Session.ts b/packages/protocol/src/session/Session.ts index 428ade234e..0f90273fb0 100644 --- a/packages/protocol/src/session/Session.ts +++ b/packages/protocol/src/session/Session.ts @@ -9,6 +9,7 @@ import { PeerLossContext } from "#peer/PeerLossContext.js"; import { SessionClosedError } from "#protocol/errors.js"; import { MessageChannel } from "#protocol/MessageChannel.js"; import type { MessageExchange } from "#protocol/MessageExchange.js"; +import { MRP } from "#protocol/MRP.js"; import { SessionIntervals } from "#session/SessionIntervals.js"; import { AsyncObservable, @@ -55,6 +56,7 @@ export abstract class Session { activeTimestamp: Timestamp = 0; abstract type: SessionType; + #peerMrpMargins?: () => MRP.Margins | undefined; #closing = ObservableValue(); #gracefulClose = AsyncObservable<[]>(); readonly #exchanges = new Set(); @@ -199,6 +201,23 @@ export abstract class Session { }; } + /** + * Additive MRP retransmission margins for the peer's network medium, or undefined if the medium is unknown. + * + * Applies to every exchange on the session, including those the peer initiates. + */ + get peerMrpMargins(): MRP.Margins | undefined { + return this.#peerMrpMargins?.(); + } + + /** + * Installs the resolver for {@link peerMrpMargins}. A resolver rather than a value because the peer's medium + * often becomes known (or changes, e.g. a new thread channel) only after the session exists. + */ + set peerMrpMarginsResolver(resolver: () => MRP.Margins | undefined) { + this.#peerMrpMargins = resolver; + } + /** * Allows updating the Session timing parameters based on received information from the peer during PASE/CASE initialization */ diff --git a/packages/protocol/test/peer/NetworkProfileTest.ts b/packages/protocol/test/peer/NetworkProfileTest.ts index 85c08b1706..71d87e938c 100644 --- a/packages/protocol/test/peer/NetworkProfileTest.ts +++ b/packages/protocol/test/peer/NetworkProfileTest.ts @@ -40,6 +40,30 @@ describe("NetworkProfiles", () => { expect(profiles.get("unknown").additionalMrpDelay).equals(Seconds(1.5)); }); + it("bdx defaults to the messaging margin", () => { + const profiles = new NetworkProfiles(); + expect(profiles.get("wifi").bdxAdditionalMrpDelay).equals(Seconds(1)); + expect(profiles.get("fast").bdxAdditionalMrpDelay).equals(Millis(0)); + expect(profiles.get("conservative").bdxAdditionalMrpDelay).equals(Seconds(1.5)); + }); + + it("thread sets bdx independently of its messaging margin", () => { + const profiles = new NetworkProfiles(); + const thread = profiles.get("thread"); + expect(thread.additionalMrpDelay).equals(Seconds(1.5)); + expect(thread.bdxAdditionalMrpDelay).equals(Millis(0)); + expect(thread.connect?.bdxAdditionalMrpDelay).equals(Millis(0)); + expect(thread.probeAddress?.bdxAdditionalMrpDelay).equals(Millis(0)); + }); + + it("a configured bdx margin overrides the default", () => { + const profiles = new NetworkProfiles(); + profiles.defaults = { wifi: { bdxAdditionalMrpDelay: Seconds(3) } }; + const wifi = profiles.get("wifi"); + expect(wifi.additionalMrpDelay).equals(Seconds(1)); + expect(wifi.bdxAdditionalMrpDelay).equals(Seconds(3)); + }); + it("connect and probe sub-profiles inherit the parent additive delay", () => { const profiles = new NetworkProfiles(); const conservative = profiles.get("conservative"); diff --git a/packages/protocol/test/peer/PeerConnectionKickTest.ts b/packages/protocol/test/peer/PeerConnectionKickTest.ts index 529034aef0..4f67a160bc 100644 --- a/packages/protocol/test/peer/PeerConnectionKickTest.ts +++ b/packages/protocol/test/peer/PeerConnectionKickTest.ts @@ -64,6 +64,34 @@ describe("PeerConnection kick redesign", () => { expect(custom.kickRestartCooldown.addressChange).equals(Minutes(5)); expect(custom.kickRestartCooldown.connect).equals(Seconds(30)); }); + + it("keeps the untouched members of a partially overridden group", () => { + const custom = PeerTimingParameters({ + kickRestartCooldown: { connect: Seconds(30) }, + addressChangeProbeCooldown: { minimum: Seconds(45) }, + }); + + expect(custom.kickRestartCooldown.connect).equals(Seconds(30)); + expect(custom.kickRestartCooldown.addressChange).equals( + PeerTimingParameters.defaults.kickRestartCooldown.addressChange, + ); + + expect(custom.addressChangeProbeCooldown.minimum).equals(Seconds(45)); + expect(custom.addressChangeProbeCooldown.maximum).equals( + PeerTimingParameters.defaults.addressChangeProbeCooldown.maximum, + ); + }); + + it("keeps the untouched members of a group when merging onto a custom base", () => { + const base = PeerTimingParameters({ + kickRestartCooldown: { addressChange: Minutes(5), connect: Minutes(5) }, + }); + + const merged = PeerTimingParameters.merge(base, { kickRestartCooldown: { connect: Seconds(30) } }); + + expect(merged.kickRestartCooldown.connect).equals(Seconds(30)); + expect(merged.kickRestartCooldown.addressChange).equals(Minutes(5)); + }); }); describe("KickOrigin through QuietObservable", () => { diff --git a/packages/protocol/test/protocol/MessageExchangeTest.ts b/packages/protocol/test/protocol/MessageExchangeTest.ts index 24d23ee951..512680124c 100644 --- a/packages/protocol/test/protocol/MessageExchangeTest.ts +++ b/packages/protocol/test/protocol/MessageExchangeTest.ts @@ -7,6 +7,7 @@ import { Message } from "#codec/MessageCodec.js"; import { NetworkProfile } from "#peer/NetworkProfile.js"; import { MessageExchange } from "#protocol/MessageExchange.js"; +import { MRP } from "#protocol/MRP.js"; import { ProtocolMocks } from "#protocol/ProtocolMocks.js"; import { SessionParameters } from "#session/SessionParameters.js"; import { Bytes, Duration, MatterFlowError, Millis, NetworkError, Seconds, Semaphore } from "@matter/general"; @@ -260,7 +261,12 @@ describe("MessageExchange", () => { describe("MRP backoff margin", () => { // A throttle profile whose own additionalMrpDelay is deliberately wrong; the exchange must ignore it. function unlimitedThrottle(): NetworkProfile { - return { id: "unlimited", semaphore: new Semaphore("test", Infinity), additionalMrpDelay: Seconds(5) }; + return { + id: "unlimited", + semaphore: new Semaphore("test", Infinity), + additionalMrpDelay: Seconds(5), + bdxAdditionalMrpDelay: Seconds(5), + }; } // Captures the additionalDelay the exchange passes to the channel, then aborts the send before it awaits @@ -269,6 +275,9 @@ describe("MessageExchange", () => { localAdditionalMrpDelay: Duration; localFixedMrpBackoff?: Duration; peerAdditionalMrpDelay?: Duration; + sessionPeerMrpMargins?: MRP.Margins; + peerInitiated?: boolean; + protocolId?: number; network?: NetworkProfile; }): Promise<{ additionalDelay?: Duration; fixedBackoff?: Duration }> { // MRP only engages on unreliable transports; the default mock channel is reliable. @@ -288,19 +297,27 @@ describe("MessageExchange", () => { throw new NetworkError("captured"); }; - const exchange = MessageExchange.initiate( - { - session, - localSessionParameters: SessionParameters(SessionParameters.defaults), - localAdditionalMrpDelay: options.localAdditionalMrpDelay, - localFixedMrpBackoff: options.localFixedMrpBackoff ?? Millis(0), - async peerLost() {}, - retry() {}, - }, - 1, - SECURE_CHANNEL_PROTOCOL_ID, - { network: options.network, peerAdditionalMrpDelay: options.peerAdditionalMrpDelay }, - ); + if (options.sessionPeerMrpMargins !== undefined) { + session.peerMrpMarginsResolver = () => options.sessionPeerMrpMargins; + } + + const context = { + session, + localSessionParameters: SessionParameters(SessionParameters.defaults), + localAdditionalMrpDelay: options.localAdditionalMrpDelay, + localFixedMrpBackoff: options.localFixedMrpBackoff ?? Millis(0), + async peerLost() {}, + retry() {}, + }; + const exchangeOptions = { + network: options.network, + peerAdditionalMrpDelay: options.peerAdditionalMrpDelay, + }; + + const protocolId = options.protocolId ?? SECURE_CHANNEL_PROTOCOL_ID; + const exchange = options.peerInitiated + ? MessageExchange.fromInitialMessage(context, fakeInboundMessage({ protocolId }), exchangeOptions) + : MessageExchange.initiate(context, 1, protocolId, exchangeOptions); await expect(exchange.send(1, Bytes.empty, { requiresAck: true })).to.be.rejectedWith("captured"); return captured; @@ -336,6 +353,57 @@ describe("MessageExchange", () => { expect(captured.additionalDelay).equals(Millis(0)); }); + it("applies the session's peer margin to peer-initiated exchanges", async () => { + const captured = await captureAdditionalDelay({ + localAdditionalMrpDelay: Millis(0), + sessionPeerMrpMargins: { messaging: Seconds(1.5), bdx: Seconds(1.5) }, + peerInitiated: true, + }); + + expect(captured.additionalDelay).equals(Seconds(1.5)); + }); + + it("applies the session's peer margin to initiated exchanges without an explicit margin", async () => { + const captured = await captureAdditionalDelay({ + localAdditionalMrpDelay: Millis(0), + sessionPeerMrpMargins: { messaging: Seconds(1.5), bdx: Seconds(1.5) }, + }); + + expect(captured.additionalDelay).equals(Seconds(1.5)); + }); + + it("prefers the explicit peer margin over the session's", async () => { + const captured = await captureAdditionalDelay({ + localAdditionalMrpDelay: Millis(0), + peerAdditionalMrpDelay: Millis(0), + sessionPeerMrpMargins: { messaging: Seconds(1.5), bdx: Seconds(1.5) }, + }); + + expect(captured.additionalDelay).equals(Millis(0)); + }); + + it("selects the BDX margin for BDX exchanges, in either direction", async () => { + for (const peerInitiated of [false, true]) { + const captured = await captureAdditionalDelay({ + localAdditionalMrpDelay: Millis(0), + sessionPeerMrpMargins: { messaging: Seconds(1.5), bdx: Seconds(5) }, + protocolId: BDX_PROTOCOL_ID, + peerInitiated, + }); + + expect(captured.additionalDelay).equals(Seconds(5)); + } + }); + + it("selects the messaging margin for other protocols", async () => { + const captured = await captureAdditionalDelay({ + localAdditionalMrpDelay: Millis(0), + sessionPeerMrpMargins: { messaging: Seconds(1.5), bdx: Seconds(5) }, + }); + + expect(captured.additionalDelay).equals(Seconds(1.5)); + }); + it("passes localFixedMrpBackoff through as the fixed backoff pad, separate from additionalDelay", async () => { const captured = await captureAdditionalDelay({ localAdditionalMrpDelay: Millis(0),