Skip to content

js: strict receiver validation per BBQr BIP draft - #22

Open
scgbckbone wants to merge 4 commits into
coinkite:masterfrom
scgbckbone:bip-compat-js
Open

scgbckbone wants to merge 4 commits into
coinkite:masterfrom
scgbckbone:bip-compat-js

Conversation

@scgbckbone

Copy link
Copy Markdown
Contributor

No description provided.

@scgbckbone scgbckbone left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@siim-m please check this "vibe"

@nvk nvk left a comment

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 existing JavaScript suite and build pass (19 tests), and I also checked the draft's 4 positive and 13 negative vectors. The parser changes mostly behave as intended, but the Z path still has two blocking conformance/security gaps. Please also add regression tests for the new validation branches; this PR currently changes parser behavior without adding tests.

Comment thread js/src/utils.ts Outdated
Comment thread js/src/utils.ts
@scgbckbone

Copy link
Copy Markdown
Contributor Author

Both P1s addressed in f5ebcb5. Tests are now committed: js/tests/vectors.test.ts runs the draft's 4 positive vectors (forward + reverse order), all 14 negative vectors, plus the two new Z regression cases. Full suite: 39 passed, tsc clean. Same hardening + tests applied to the python PR (#21).

@nvk nvk left a comment

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 streaming 16 MiB output cap is fixed, and the expanded suite/build pass (39 tests, TypeScript and Vite clean). Two conformance issues remain: the 1 KiB distance bound is still approximate rather than exact, and the public BIP vector file has reintroduced a duplicate-policy case that draft v4 intentionally removed from the normative negative set.

Comment thread test_data/bip-test-vectors.json Outdated

@siim-m siim-m left a comment

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.

@scgbckbone I can't seem to find the actual BIP draft anywhere, so cannot really review against the spec itself. But from what I can infer, just a few minor things.

Comment thread js/src/utils.ts
Comment thread js/src/join.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants