Skip to content

node(svm-watcher): Add type to validate message account data#4657

Open
johnsaigle wants to merge 5 commits intowormhole-foundation:mainfrom
johnsaigle:solana-watcher-more-checks
Open

node(svm-watcher): Add type to validate message account data#4657
johnsaigle wants to merge 5 commits intowormhole-foundation:mainfrom
johnsaigle:solana-watcher-more-checks

Conversation

@johnsaigle
Copy link
Copy Markdown
Contributor

@johnsaigle johnsaigle commented Feb 4, 2026 •
edited
Loading

  • Add new MessageAcccountData type to validate message account data is the right size and has a valid prefix.
  • Modify relevant function signatures to accept the new type instead of []byte.
  • Modify the recent changes to the SVM watcher's unit test by using the new safe struct instead of manually building the message account data.

@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 282953b to 22a66cb Compare February 4, 2026 18:49
@johnsaigle johnsaigle marked this pull request as ready for review February 4, 2026 18:49
djb15
djb15 reviewed Feb 6, 2026
View reviewed changes
Comment thread node/pkg/watchers/solana/client.go Outdated
Comment thread node/pkg/watchers/solana/client_test.go Outdated
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch 2 times, most recently from 601307f to 1533f7a Compare February 9, 2026 15:02
@johnsaigle johnsaigle requested a review from djb15 February 9, 2026 15:02
djb15
djb15 previously approved these changes Feb 13, 2026
View reviewed changes
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch 2 times, most recently from bba20de to 30e833d Compare February 13, 2026 20:57
mdulin2
mdulin2 previously approved these changes Feb 17, 2026
View reviewed changes
Comment thread node/pkg/watchers/solana/client.go
Comment thread node/pkg/watchers/solana/client.go
@johnsaigle johnsaigle dismissed stale reviews from mdulin2 and djb15 via cbaf116 February 17, 2026 14:13
@johnsaigle johnsaigle mentioned this pull request Feb 20, 2026
Merged
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 4889df9 to be6b5d6 Compare February 20, 2026 13:36
djb15
djb15 previously approved these changes Feb 20, 2026
View reviewed changes
pleasew8t
pleasew8t reviewed Feb 24, 2026
View reviewed changes
Comment thread node/pkg/watchers/solana/client.go Outdated
pleasew8t
pleasew8t previously approved these changes Feb 24, 2026
View reviewed changes
@johnsaigle johnsaigle dismissed stale reviews from pleasew8t and djb15 via 0665fd5 February 24, 2026 13:34
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from be6b5d6 to 0665fd5 Compare February 24, 2026 13:34
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 0665fd5 to d3fa447 Compare February 25, 2026 16:14
djb15
djb15 previously approved these changes Mar 10, 2026
View reviewed changes
@johnsaigle johnsaigle mentioned this pull request Mar 11, 2026
Merged
@johnsaigle johnsaigle marked this pull request as draft March 11, 2026 19:01
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from d3fa447 to 074ff88 Compare March 12, 2026 12:23
@johnsaigle johnsaigle marked this pull request as ready for review March 17, 2026 12:34
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 074ff88 to 41fae94 Compare March 17, 2026 12:34
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 41fae94 to 2b6d257 Compare March 24, 2026 15:12
@johnsaigle johnsaigle requested a review from djb15 March 24, 2026 15:12
djb15
djb15 previously approved these changes Mar 24, 2026
View reviewed changes
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 2b6d257 to 48a2470 Compare March 24, 2026 17:50
@johnsaigle johnsaigle marked this pull request as draft April 10, 2026 14:52
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 48a2470 to 191aa10 Compare April 15, 2026 12:43
@johnsaigle johnsaigle marked this pull request as ready for review April 15, 2026 12:44
@johnsaigle
Copy link
Copy Markdown
Contributor Author

johnsaigle commented Apr 15, 2026 •
edited
Loading

@djb15 @pleasew8t @mdulin2 could you please re-review? the only changes since your previous approvals are the ones in the unit test file

djb15
djb15 reviewed Apr 15, 2026
View reviewed changes
Comment thread node/pkg/watchers/solana/client_test.go
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 20d29f4 to 477b0d6 Compare April 17, 2026 11:12
@johnsaigle johnsaigle requested review from djb15 and mdulin2 April 17, 2026 11:12
djb15
djb15 previously approved these changes Apr 17, 2026
View reviewed changes
@johnsaigle
node(svm-watcher): Add type to validate message account data
18ac7bd 
- Add new MessageAcccountData type to validate message account data
is the right size and has a valid prefix.
- Modify relevant function signatures to accept the new type instead of []byte.

Add unit tests for parsing

More comments

revert gap bytes changes

decouple prefix length from min account data; test on specific err strings

add test for short prefix

Add check in the programSubscribe path

embed slice access in else

Add security comment for parsing account data; fix unit test

Use copy for new struct; log on empty account data

Add String and Bytes to clarify access pattern for data

Add String and Bytes to clarify access pattern for data

refactor to use struct,err return instead of nil-able pointer

return errors from constructor

fix wrong err return; avoid double copy

fix regression in err handling; add test for constructor

Add mutability test; explain length check; fix logs

cleanup after rebase

make comment more precise
@johnsaigle
stitch things together after rebase
3d7ce0c
@johnsaigle
add comment and err string matching for ambiguous test
4cc16bd
@johnsaigle johnsaigle force-pushed the solana-watcher-more-checks branch from 477b0d6 to 4cc16bd Compare April 20, 2026 11:36
@johnsaigle johnsaigle requested a review from kcsongor as a code owner April 20, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djb15 djb15 djb15 left review comments

@bemic bemic Awaiting requested review from bemic bemic is a code owner

@evan-gray evan-gray Awaiting requested review from evan-gray evan-gray is a code owner

@panoel panoel Awaiting requested review from panoel panoel is a code owner

@fergarrui fergarrui Awaiting requested review from fergarrui

@pleasew8t pleasew8t Awaiting requested review from pleasew8t pleasew8t is a code owner

@mdulin2 mdulin2 Awaiting requested review from mdulin2 mdulin2 is a code owner

@kcsongor kcsongor Awaiting requested review from kcsongor kcsongor is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@johnsaigle @pleasew8t @djb15 @mdulin2