-
Notifications
You must be signed in to change notification settings - Fork 278
fix(protocol-devtools): treat explicitly-empty ULN config values as NIL sentinels #1944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+1,434
−205
Merged
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
fb9da3b
fix(protocol-devtools): treat explicitly-empty ULN config values as N…
St0rmBr3w c6a92e0
fix(protocol-devtools): address PR review on ULN NIL sentinels
St0rmBr3w cebc3e9
fix(devtools): complete ULN NIL round-trip for required DVNs + run So…
St0rmBr3w 6f8e19e
fix(protocol-devtools-evm): complete ULN NIL round-trip for the Read …
St0rmBr3w cc068e2
refactor(protocol-devtools): hoist shared ULN empty->NIL resolution i…
St0rmBr3w 8d97cde
fix(lzapp-migration): guard encodeUlnConfig against omitted DVN arrays
St0rmBr3w 998bc10
refactor(protocol-devtools): make requiredDVNs optional, unify DVN co…
St0rmBr3w a108405
fix(lzapp-migration): default scalar ULN fields in encodeUlnConfig
St0rmBr3w 5c3d31c
docs(metadata-tools): changeset for empty optionalDVNs pinning NIL
St0rmBr3w 3578560
docs(changeset): reframe NIL semantics around team-controlled config
St0rmBr3w 8a43cf7
fix(protocol-devtools-evm): reject empty requiredDVNs on the default …
St0rmBr3w 2efdd21
test(ua-devtools-evm-hardhat): cover the ULN config generators
St0rmBr3w 05efccd
fix(protocol-devtools-evm): allow optional-only default ULN config
St0rmBr3w 14accc4
docs(lzapp-migration): document encodeUlnConfig resolved-config invar…
St0rmBr3w b97eb44
fix(devtools-move): guard against omitted requiredDVNs in buildConfig
St0rmBr3w cda3f1b
fix(devtools-move): reject omitted requiredDVNs instead of pinning NIL
St0rmBr3w e291989
refactor(protocol-devtools): hoist ULN threshold/default/generator lo…
St0rmBr3w d49c45b
docs(examples): explain the DVN config tuple in oft / oft-solana configs
St0rmBr3w 61761a6
test(protocol-devtools-solana): cover requiredDVNs sentinel mapping
St0rmBr3w a7c1d41
test(ua-devtools-evm-hardhat): match generator field omission in expe…
St0rmBr3w 53ff395
docs(oft-example): clarify optionalDVNs pin in simple-workers-mock
St0rmBr3w e1bbdc1
test(ua-devtools-evm-hardhat): note expected-config helpers assume se…
St0rmBr3w 61b9e5c
refactor(protocol-devtools): review nits — guard clauses, naming, dea…
St0rmBr3w f348eb7
Merge branch 'main' into krak/fix-uln-nil-sentinels
St0rmBr3w File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| --- | ||
| "@layerzerolabs/protocol-devtools": major | ||
| "@layerzerolabs/protocol-devtools-evm": major | ||
| "@layerzerolabs/protocol-devtools-solana": major | ||
| --- | ||
|
|
||
| Treat explicitly-empty ULN302 config values as NIL sentinels instead of defaults | ||
|
|
||
| When serializing an OApp ULN302 config, an explicitly-empty field now pins the | ||
| literal zero/none via the protocol's NIL sentinel, while an omitted field still | ||
| inherits the on-chain default: | ||
|
|
||
| - `confirmations: 0n` now serializes to `NIL_CONFIRMATIONS` (`type(uint64).max`). | ||
| - `optionalDVNs: []` now serializes to `NIL_DVN_COUNT` (`0xff`), matching the | ||
| existing behavior of `requiredDVNs: []`. | ||
| - Omitting a field (leaving it `undefined`) continues to inherit the on-chain | ||
| default. | ||
|
|
||
| To support this, `Uln302UlnConfig`/`UlnReadUlnConfig` now carry `optionalDVNCount` | ||
| (and `UlnReadUlnConfig` also carries `requiredDVNCount`) so the stored sentinel | ||
| round-trips through the configuration diff. The on-chain read path no longer | ||
| re-applies the empty→NIL mapping, keeping `hasAppUlnConfig` idempotent. The | ||
| library-wide DEFAULT config continues to serialize literal values (it rejects NIL | ||
| sentinels on-chain). On Solana, `confirmations` is now encoded as a `BN` so the | ||
| `u64` NIL sentinel survives without precision loss. | ||
|
|
||
| MIGRATION: if you wrote `confirmations: 0` or `optionalDVNs: []` expecting the | ||
| config to inherit the protocol default, OMIT the field instead. An explicit empty | ||
| value now pins literal zero/none — for `confirmations` this means zero block | ||
| confirmations, which is security-relevant. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encodeUlnConfig ignores chain semantics
High Severity
encodeUlnConfigstill runs every input through user-orientedserializeUlnConfig, so chain-shaped configs (e.g. fromgetAppUlnConfigor decode→encode) are mis-encoded: stored inheritconfirmationsof0becomesNIL_CONFIRMATIONS, andoptionalDVNCount: 0withoptionalDVNs: []becomesNIL_DVN_COUNTbecauseoptionalDVNCountis not honored likerequiredDVNCount.Additional Locations (1)
packages/protocol-devtools-solana/src/uln302/sdk.ts#L187-L249Reviewed by Cursor Bugbot for commit fb9da3b. Configure here.