(closed) by Shitikyan · Pull Request #892 · kynesyslabs/node

Shitikyan · 2026-06-01T08:56:05Z

No description provided.

coderabbitai · 2026-06-01T08:56:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5f5e865d-19c0-44db-97e0-84986921b791

📥 Commits

Reviewing files that changed from the base of the PR and between 924df40 and 372ed10.

📒 Files selected for processing (1)

test-reports/findings-2026-05-31.md

Walkthrough

A new test-report markdown document consolidates end-to-end findings from dev environment testing, covering L2PS encrypted-broadcast security issues (nonce-reuse and pending requirements), governance-feature failure (hash-mismatch with diagnostics), test-coverage gaps, nonce-enforcement discoveries, UX improvements, and operational risks blocking rollout.

Changes

L2PS and Governance End-to-End Findings

Layer / File(s)	Summary
Report header and scope `test-reports/findings-2026-05-31.md`	Document title, metadata (author, test period, dev environment target, tracking ticket), and scope statement organizing findings by topic.
L2PS encryption security findings `test-reports/findings-2026-05-31.md`	AES-GCM nonce-reuse vulnerability in encrypted-broadcast, SDK fix deploying fresh nonce per `encryptTx` with legacy decrypt fallback, and three outstanding security requirements pending production rollout.
Governance hash-mismatch failure and diagnostics `test-reports/findings-2026-05-31.md`	End-to-end upgradable-network failure on `dev.node2` due to transaction hash mismatch, known SDK↔node serializer asymmetries, dirty-state condition on deployed node, non-repro on local devnet, landed diagnostic/test items, and remaining remediation plan.
Per-transaction nonce enforcement side findings `test-reports/findings-2026-05-31.md`	Missing per-transaction nonce enforcement on node identified; four merged node PRs introducing nonce enforcement with per-sender locking to close TOCTOU window, with pending follow-up draft batch item.
UX improvements and operational status `test-reports/findings-2026-05-31.md`	Agent-commerce UX improvement to surface L2PS confirmation via badge/pill for successful encrypted broadcast; summary of remaining operational risks, blocked rollout conditions, and Linear ticket status map for tracked issues.
Source material and references `test-reports/findings-2026-05-31.md`	Investigation source links: battery output, serializer asymmetry analysis, and supporting Telegram thread.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Consolidated findings report #891: The PR implements the exact deliverable of adding a consolidated L2PS and upgradable-network findings report as requested.
Governance + L2PS — May 2026 PR batch #877: The findings document directly describes the same security investigation and implementation PRs (SDK nonce fix, governance diagnostics, UX badge) tracked in this issue.
Node — investigation aids for governance hash mismatch #879: The test report documents governance hash-mismatch diagnostics (battery script and isCoherent dump) implemented in related governance fix work.

Poem

🐰 A findings scroll, so thorough and clear,
Documents security and governance's fear!
Nonce-reuse fixed, and serializers aligned,
Test gaps now mapped, improvements designed.
Rollout awaits as the path becomes kind! 🚀

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: consolidating L2PS and Upgradable Network findings from testing into a single report document.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch docs/l2ps-governance-findings-report

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

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

qodo-code-review · 2026-06-01T08:56:21Z

Review Summary by Qodo

(Agentic_describe updated until commit `372ed10`)

L2PS and Upgradable Network Testing Findings Report

📝 Documentation

Walkthroughs

Description

• Consolidated findings from L2PS encryption and governance testing pass
• Documents nonce reuse vulnerability, governance hash mismatch, and test coverage gaps
• Maps security fixes, pending work, and operational risks across multiple repos
• Provides cross-linked source material and Linear ticket tracking

Diagram

flowchart LR
  Testing["Testing Pass<br/>2026-05-26 to 31"]
  L2PS["L2PS Nonce Reuse<br/>SDK #87 merged"]
  Gov["Governance Hash<br/>Mismatch on dev.node2"]
  Coverage["Test Coverage Gap<br/>SDK #90 merged"]
  Nonce["Nonce Enforcement<br/>Node #884-887 merged"]
  UX["L2PS UI Badge<br/>Demo #11 open"]
  
  Testing --> L2PS
  Testing --> Gov
  Testing --> Coverage
  Testing --> Nonce
  Testing --> UX
  
  L2PS -- "3 follow-ups pending" --> Gov
  Gov -- "Real fix DEM-727" --> Coverage
  Coverage -- "Integration test gap" --> Nonce

File Changes

1. test-reports/findings-2026-05-31.md 📝 Documentation +152/-0

Consolidated L2PS and governance testing findings report

• New comprehensive findings report consolidating L2PS encryption and governance testing results
• Documents five major findings: nonce reuse vulnerability, governance hash mismatch, test coverage
 gaps, nonce enforcement, and UX improvements
• Includes operational risks, Linear ticket map, and cross-links to source material and related PRs
• Covers testing period 2026-05-26 to 2026-05-31 against dev.node2 (v0.9.8)

test-reports/findings-2026-05-31.md

qodo-code-review · 2026-06-01T08:56:22Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0)

1. Orphaned report location 🐞 Bug ⚙ Maintainability ⭐ New

Description

The new findings report is added under a new top-level test-reports/ directory that isn’t part of
the repo’s existing test-report workflow (which is built around testing/runs). This makes the
report easy to miss and harder to maintain/discover alongside the rest of the testing artifacts.

Code

test-reports/findings-2026-05-31.md[1]

Evidence

The report is placed in a new test-reports/ folder, while existing tooling for surfacing reports
reads from testing/runs/_latest, and .gitignore documents testing/runs/ as the place for test
artifacts—indicating test-reports/ is not part of the established pattern.

test-reports/findings-2026-05-31.md[1-8]
testing/scripts/show-latest-report.ts[6-8]
.gitignore[88-93]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A human-authored findings report was added under `test-reports/`, but the repo’s existing test-reporting convention/tooling uses `testing/runs` (and related scripts). This makes the report effectively “orphaned” from the repo’s established workflows and discoverability.

### Issue Context
- This is a documentation-only PR.
- The goal is to ensure the report lives where future contributors will naturally look, or that the standard tooling/index points to it.

### Fix Focus Areas
- test-reports/findings-2026-05-31.md[1-152]
- testing/scripts/show-latest-report.ts[6-8]
- .gitignore[88-93]

### What to change
- Either relocate the report under an existing documentation/tests artifact area (e.g., `documentation/` or `testing/runs/` if it’s meant to be treated as a test run artifact).
- Or keep the file where it is, but add a small index/README or a pointer from an established place (e.g., a `documentation/` index or a `testing/` README) so it’s discoverable via existing navigation/workflows.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Unarchived chat source 🐞 Bug ⚙ Maintainability ⭐ New

Description

The report cites a Telegram thread as a source of truth, but that content is not captured in-repo,
making the report’s provenance non-auditable from the repository alone. This reduces long-term
traceability because the referenced discussion can’t be reviewed/verified via version control.

Code

test-reports/findings-2026-05-31.md[152]

Evidence

The report explicitly lists a Telegram team chat thread as source material, which is outside the
repository and therefore not reviewable/auditable from this branch alone.

test-reports/findings-2026-05-31.md[152-152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The report references a Telegram thread as source material, but the repo does not contain an archived copy. That makes the report less useful for future readers who rely on the repository history to audit decisions and context.

### Issue Context
The report is otherwise a useful consolidation of findings; capturing externally-hosted discussion as a short appendix in-repo keeps the report self-contained and reviewable.

### Fix Focus Areas
- test-reports/findings-2026-05-31.md[148-152]

### What to change
- Replace the Telegram-thread bullet with an archived, sanitized summary committed as a markdown file in the repo (or inline an appendix section), and link to that artifact.
- Keep the Telegram reference if desired, but make the repo artifact the primary citation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Broken source-material links 🐞 Bug ⚙ Maintainability

Description

The report links to dev-node-battery-FINAL.md and governance-hash-mismatch-analysis.md as sibling
files, but they are not added in this PR/branch so the links will resolve to 404 on GitHub and the
cited evidence becomes inaccessible from the report.

Code

test-reports/findings-2026-05-31.md[R148-151]

Evidence

The report’s “Source material” section uses relative links that assume the two referenced markdown
files exist alongside the report; since the PR only introduces this one file, those links won’t
resolve in the merged branch unless the referenced files are also present or the links are updated.

test-reports/findings-2026-05-31.md[148-151]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`test-reports/findings-2026-05-31.md` contains relative links to `dev-node-battery-FINAL.md` and `governance-hash-mismatch-analysis.md`, but those files are not present in this PR branch. As a result, the report’s “Source material” links will be broken after merge.
### Issue Context
The report is intended to be the single consolidated reference; broken internal links undermine that goal.
### Fix Focus Areas
- test-reports/findings-2026-05-31.md[41-52]
- test-reports/findings-2026-05-31.md[148-151]
### What to change
Do one of the following:
1. **Make the PR self-contained**: add/copy `dev-node-battery-FINAL.md` and `governance-hash-mismatch-analysis.md` into the expected location (same directory as the report, or adjust paths accordingly).
2. **Use stable absolute links**: change the relative links to GitHub URLs pointing at the specific PR/branch/commit where those artifacts live.
3. **Document the dependency explicitly**: if you intentionally depend on another PR landing first, add a prominent note near the top of the report and link to that PR, and consider pointing links directly to that PR so they work immediately.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit 372ed10

Results up to commit 372ed10

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 (UI revamp) txeditor/xm 🎫 Rewrite demos.ts to work with HTTP 🎫 Implement HTTP logic in SDK

1. Broken source-material links 🐞 Bug ⚙ Maintainability

Description

The report links to dev-node-battery-FINAL.md and governance-hash-mismatch-analysis.md as sibling
files, but they are not added in this PR/branch so the links will resolve to 404 on GitHub and the
cited evidence becomes inaccessible from the report.

Code

test-reports/findings-2026-05-31.md[R148-151]

Evidence

The report’s “Source material” section uses relative links that assume the two referenced markdown
files exist alongside the report; since the PR only introduces this one file, those links won’t
resolve in the merged branch unless the referenced files are also present or the links are updated.

test-reports/findings-2026-05-31.md[148-151]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`test-reports/findings-2026-05-31.md` contains relative links to `dev-node-battery-FINAL.md` and `governance-hash-mismatch-analysis.md`, but those files are not present in this PR branch. As a result, the report’s “Source material” links will be broken after merge.

### Issue Context
The report is intended to be the single consolidated reference; broken internal links undermine that goal.

### Fix Focus Areas
- test-reports/findings-2026-05-31.md[41-52]
- test-reports/findings-2026-05-31.md[148-151]

### What to change
Do one of the following:
1. **Make the PR self-contained**: add/copy `dev-node-battery-FINAL.md` and `governance-hash-mismatch-analysis.md` into the expected location (same directory as the report, or adjust paths accordingly).
2. **Use stable absolute links**: change the relative links to GitHub URLs pointing at the specific PR/branch/commit where those artifacts live.
3. **Document the dependency explicitly**: if you intentionally depend on another PR landing first, add a prominent note near the top of the report and link to that PR, and consider pointing links directly to that PR so they work immediately.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

greptile-apps · 2026-06-01T08:57:51Z

Greptile Summary

This PR adds a findings report (test-reports/findings-2026-05-31.md) consolidating everything surfaced during the L2PS encrypted-broadcast and upgradable-network (governance) testing pass against dev.node2 over 2026-05-26 → 2026-05-31.

L2PS nonce reuse (section 1): documents the AES-GCM static-IV break fixed in SDK PR Implement HTTP logic in SDK #87, plus three security hardening items (nonce-as-additionalData binding, mirroring on decrypt, strict-undefined wire-nonce validation) that must land before production rollout — tracked in DEM-725.
Governance hash mismatch (sections 2–3): documents the SDK ↔ node serializer investigation, the diagnostic-only Node PR diag(governance): dev-node battery + isCoherent mismatch dump + hash-mismatch analysis #876, and the real-fix work plan in DEM-727; SDK PR Rewrite demos.ts to work with HTTP #90 (merged) closes the SDK-side canonical-bytes gap.
Nonce enforcement (section 4) and operational risks (section 6): captures the four-PR nonce batch (PRs feat(forks): register nonceEnforcement fork + ship assignNonce validation infra (batch C PR 1) #884–fix(mempool): close same-node nonce TOCTOU via per-sender advisory lock (batch C PR 4) #887, merged) and the remaining open items including the dirty dev.node2 binary and the missing SDK→devnet integration test.

Confidence Score: 4/5

Safe to merge as a standalone report, but the document links to two files that do not yet exist in the repository, leaving readers unable to navigate to the key serializer analysis and battery run output referenced throughout.

The findings report itself is well-structured and accurate, but it has a concrete defect: relative links to governance-hash-mismatch-analysis.md and dev-node-battery-FINAL.md in sections 2 and 8 resolve to nowhere because Node PR #876 — which was supposed to ship those files — is still open. Merging this report before those files land means every reader who follows the most important source references hits a dead end.

test-reports/findings-2026-05-31.md — specifically the relative-path links to the battery report and serializer analysis in sections 2 and 8.

Important Files Changed

Filename	Overview
test-reports/findings-2026-05-31.md	New findings report covering L2PS nonce-reuse fix, governance hash-mismatch investigation, nonce enforcement, and open operational risks — contains broken relative links to two source-material files (battery report and serializer analysis) whose parent PR (#876) is still unmerged.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[findings-2026-05-31.md] --> B[L2PS Nonce Reuse\nDEM-725]
    A --> C[Governance Hash Mismatch\nDEM-727]
    A --> D[Nonce Enforcement\nDEM-724 area]
    A --> E[Operational Risks\nDEM-728]

    B --> B1[SDK PR #87 landed\nfresh nonce per encryptTx]
    B --> B2[3 security follow-ups\nstill open — blocks prod]

    C --> C1[SDK PR #90 merged\nroundTripHash fixtures]
    C --> C2[Node PR #876 open\ndiagnostics only]
    C --> C3[Real fix pending\nDEM-727 Todo]

    D --> D1[PRs #884-#887 merged\nnonce validation + TOCTOU lock]
    D --> D2[PR #888 DRAFT\nvote-race BFT fix]

    E --> E1[dev.node2 dirty=true\nDEM-728]
    E --> E2[No SDK->devnet\nintegration test for governance]

    style B2 fill:#ffcccc
    style C3 fill:#ffcccc
    style D2 fill:#fff3cd
    style E1 fill:#fff3cd
    style E2 fill:#fff3cd

_{Reviews (2): Last reviewed commit: 924df40 | Re-trigger Greptile}

greptile-apps · 2026-06-01T08:57:55Z

+| [DEM-726](https://linear.app/kynesys-labs/issue/DEM-726) | In Review | Demo #11 — L2PS UI badge |
+| [DEM-727](https://linear.app/kynesys-labs/issue/DEM-727) | Todo | Reproduce governance hash mismatch locally + ship real fix |
+| [DEM-728](https://linear.app/kynesys-labs/issue/DEM-728) | Todo | Re-run battery after devnet reboot |
+| [DEM-729](https://linear.app/kynesys-labs/issue/DEM-729) | Todo | This report |


DEM-729 is listed as "Todo" in the ticket map even though this report is its deliverable and the PR description says it closes DEM-729. Marking it as "Done" (or "In Progress" if the ticket hasn't been closed yet) avoids confusion for anyone who reads the report after merge.

Suggested change

| [DEM-729](https://linear.app/kynesys-labs/issue/DEM-729) | Todo | This report |

| [DEM-729](https://linear.app/kynesys-labs/issue/DEM-729) | Done | This report |

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

qodo-code-review · 2026-06-01T09:19:12Z

Code review by qodo was updated up to the latest commit 372ed10

greptile-apps Bot reviewed Jun 1, 2026

View reviewed changes

linear Bot mentioned this pull request Jun 1, 2026

Consolidated findings report #891

Closed

Shitikyan closed this Jun 1, 2026

Shitikyan deleted the docs/l2ps-governance-findings-report branch June 1, 2026 09:12

Shitikyan changed the title ~~docs(test-reports): consolidated L2PS + Upgradable Network findings (per Hovhannes request)~~ (closed) Jun 1, 2026

Shitikyan reopened this Jun 1, 2026

Shitikyan closed this Jun 1, 2026

Shitikyan force-pushed the docs/l2ps-governance-findings-report branch from 372ed10 to 924df40 Compare June 1, 2026 09:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(closed)#892

(closed)#892
Shitikyan wants to merge 0 commit into
stabilisationfrom
docs/l2ps-governance-findings-report

Shitikyan commented Jun 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Jun 1, 2026

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	\| [DEM-729](https://linear.app/kynesys-labs/issue/DEM-729) \| Todo \| This report \|
	\| [DEM-729](https://linear.app/kynesys-labs/issue/DEM-729) \| Done \| This report \|

Conversation

Shitikyan commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary by Qodo

(Agentic_describe updated until commit 372ed10)

Walkthroughs

File Changes

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

greptile-apps Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Shitikyan commented Jun 1, 2026 •

edited

Loading

coderabbitai Bot commented Jun 1, 2026 •

edited

Loading

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading

(Agentic_describe updated until commit `372ed10`)

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading

greptile-apps Bot commented Jun 1, 2026 •

edited

Loading

qodo-code-review Bot commented Jun 1, 2026 •

edited

Loading