MF3-I12, I13, I14: nitronode audit remediations

[philanton]

Remediates three audit findings in nitronode. All informational/defense-in-depth — no exploit in the current healthy flow; each hardens an implicit invariant to fail closed.

MF3-I12 — channel session-key asset validation

SubmitState for channel session keys only length-checked coreState.Assets. Now validateSessionKeyAssets rejects, per asset:

• non-canonical (asset != strings.ToLower(asset)) — assets are part of the user-signed metadata hash, so the node rejects rather than rewrites, keeping the signed payload identical to what is stored;
• duplicates (after normalization);
• unsupported (GetAssetDecimals errors).

Empty asset lists still allowed. Runs before signature validation/storage.

MF3-I13 — reject submit_state on a stateless active channel

SubmitState's default branch silently bootstrapped a synthetic Void state when no stored state existed for an active home channel. An active channel always has its initial state stored atomically by request_creation, so a nil state is an invariant violation, not a first-submit. Now fails closed with active channel has no stored state. The NewVoidState fallback stays in the channel-creation path.

MF3-I14 — assert user_balances update affects one row

StoreUserState checked only the GORM Error from the user_balances Updates() and ignored RowsAffected. A caller that stored a state without first calling LockUserState would silently match zero rows, leaving channel_states and the cached balance divergent without failing the transaction. Now requires exactly one affected row. Tests that used StoreUserState without locking are updated to lock-first via a storeLocked helper, plus a negative test for the new error.

All production call sites already lock-before-store / store-initial-state, so no behavior change in the healthy flow.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ed8fb7fa-0e95-4949-83ae-b9c8fd41da57

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/mf3-i1s

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ihsraham]

Thanks Anton. I-13 and I-14 look closed to me, and the new I-12 per-asset checks for canonical, duplicate, and unsupported assets are the right direction. I am requesting changes on the remaining I-12 case because the audit also required empty asset lists to be accepted only as revocation/deactivation states.

[ihsraham]

This leaves one I-12 case open. The audit asked for assets to be empty only when expires_at <= now, so an empty list is an explicit revocation/deactivation state. With this helper taking only assets, a signed state with assets: [] and a future expires_at still passes validation and can become the active current version while authorizing no usable asset.

Before approval, please reject len(coreState.Assets) == 0 when coreState.ExpiresAt is in the future, and add the matching regression test. The duplicate, canonical, and supported-asset checks already look good.