-
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 22 commits
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,9 @@ | ||
| --- | ||
| "@layerzerolabs/devtools-move": patch | ||
| --- | ||
|
|
||
| Reject an omitted `requiredDVNs` in `buildConfig` with a clear error. `requiredDVNs` is now | ||
| optional on the shared `Uln302UlnUserConfig` type, but this encoder maps an empty required set | ||
| to the NIL sentinel (pin "no required DVNs") and cannot express "inherit the on-chain default". | ||
| Defaulting an omitted value to `[]` would silently pin the least-secure shape, so it now throws | ||
| instead — callers must pass the required DVNs explicitly, or `[]` to pin "no required DVNs". |
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,9 @@ | ||
| --- | ||
| "@layerzerolabs/ua-devtools-evm-hardhat": patch | ||
| --- | ||
|
|
||
| Generate ULN configs (both the ULN302 send/receive and the Read library generators) that | ||
| round-trip the new NIL-sentinel semantics: a field inheriting the on-chain default is | ||
| OMITTED (for both `requiredDVNs` and `optionalDVNs`, which now behave identically) rather | ||
| than emitted as an explicit empty value that would pin zero/none on re-apply. Pinned-none | ||
| configs continue to emit `[]`/`0n`. |
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,21 @@ | ||
| --- | ||
| "@layerzerolabs/metadata-tools": minor | ||
| --- | ||
|
|
||
| `generateConnectionsConfig` now treats a pathway with no optional DVNs as an explicit | ||
| "no optional DVNs" (pinned via the NIL sentinel) instead of a value that inherits the | ||
| on-chain default. | ||
|
|
||
| The emitted config still carries `optionalDVNs: []`, but under the new ULN302 sentinel | ||
| semantics that empty array now pins "no optional DVNs" on-chain rather than falling back | ||
| to the chain default. This is deliberate: the metadata config is the primary way a config | ||
| is consumed, and an empty optional-DVN set should be visible in the file rather than | ||
| silently inheriting the default. | ||
|
|
||
| Re-wiring a pathway that previously inherited the on-chain default will now pin its | ||
| optional-DVN set explicitly. If that default carried optional DVNs (a non-zero threshold), | ||
| pinning an empty set drops them — this is intended. The goal is that a team's verification | ||
| config is exactly what their config file says, not something that can change underneath them | ||
| when a LayerZero-controlled default is updated. An empty optional-DVN set means "no optional | ||
| DVNs"; teams that want an optional quorum should list those DVNs explicitly. Required DVNs | ||
| are unaffected by this change. |
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,7 @@ | ||
| --- | ||
| "@layerzerolabs/devtools-solana": minor | ||
| --- | ||
|
|
||
| Add `bigIntToBN` helper (and `Bignum` type) for converting a `bigint` to the `BN` type | ||
| the Solana program instruction builders expect, preserving full precision for `u64` | ||
| values that overflow a JS number. |
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,51 @@ | ||
| --- | ||
| "@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 | ||
|
|
||
| This lets a team pin a literal "none"/"zero" so their security configuration is exactly | ||
| what their config file says, rather than silently inheriting a default that LayerZero | ||
| controls. Being able to opt out of defaults is the point: a pinned config cannot change | ||
| underneath a team when a default is updated. | ||
|
|
||
| When serializing an OApp ULN302 / Read config, `requiredDVNs` and `optionalDVNs` now | ||
| behave identically — omitted, explicitly-empty, and concrete each map to a distinct | ||
| on-chain meaning: | ||
|
|
||
| - Omitting a DVN field (leaving it `undefined`) inherits the on-chain default. | ||
| - An explicitly-empty array (`[]`) pins "no DVNs" via `NIL_DVN_COUNT` (`0xff`). | ||
| - A concrete array pins those DVNs. | ||
| - Likewise `confirmations: 0n` now serializes to `NIL_CONFIRMATIONS` | ||
| (`type(uint64).max`), while omitting it inherits the default. | ||
|
|
||
| To make `requiredDVNs` express "inherit" the same way `optionalDVNs` already could, it | ||
| is now OPTIONAL on `Uln302UlnUserConfig` and `UlnReadUlnUserConfig` (previously | ||
| mandatory). This removes the need for any count override — the count is always derived | ||
| from the array, so the three serializers (EVM ULN302, EVM Read, Solana ULN302) share a | ||
| single `resolveDVNCount` helper. | ||
|
|
||
| The read types `Uln302UlnConfig`/`UlnReadUlnConfig` carry `optionalDVNCount` (and | ||
| `UlnReadUlnConfig` also `requiredDVNCount`) so the stored sentinel round-trips through | ||
| the configuration diff, and the on-chain read path normalizes rather than re-applying | ||
| the empty→NIL mapping, keeping `hasAppUlnConfig` idempotent on both paths. 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`, `requiredDVNs: []`, 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, and for `requiredDVNs` it means no required DVNs, both | ||
| security-relevant. Re-wiring an existing OApp whose config used these empty values | ||
| will emit a `setConfig` that flips it from inherit to pinned. | ||
| - The read types `Uln302UlnConfig` (gains `optionalDVNCount`) and `UlnReadUlnConfig` | ||
| (gains `requiredDVNCount` and `optionalDVNCount`) have new required fields. Any code | ||
| that hand-constructs one of these (e.g. mocking an SDK read) must supply the new | ||
| fields. | ||
| - `requiredDVNs` is no longer required on the user config. Code that always set it | ||
| keeps working unchanged; you may now omit it to inherit the on-chain default. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { expect } from 'chai' | ||
| import { utils } from 'ethers' | ||
| import { buildConfig } from '../tasks/evm/utils/libraryConfigUtils' | ||
| import type { Uln302UlnUserConfig } from '@layerzerolabs/toolbox-hardhat' | ||
|
|
||
| const DVN = '0x0000000000000000000000000000000000000001' | ||
| const ULN_TUPLE = [ | ||
| 'tuple(uint64 confirmations, uint8 requiredDVNCount, uint8 optionalDVNCount, uint8 optionalDVNThreshold, address[] requiredDVNs, address[] optionalDVNs)', | ||
| ] | ||
|
|
||
| const decodeRequiredDVNCount = (ulnConfigBytes: string): number => | ||
| Number(utils.defaultAbiCoder.decode(ULN_TUPLE, ulnConfigBytes)[0].requiredDVNCount) | ||
|
|
||
| describe('buildConfig requiredDVNs handling', () => { | ||
| it('throws when requiredDVNs is omitted (this path cannot inherit the default)', () => { | ||
| const config = { confirmations: BigInt(0), optionalDVNs: [], optionalDVNThreshold: 0 } as Uln302UlnUserConfig | ||
| expect(() => buildConfig(config)).to.throw('requiredDVNs must be specified') | ||
| }) | ||
|
|
||
| it('encodes an explicitly-empty requiredDVNs as the NIL sentinel (255)', () => { | ||
| const { ulnConfigBytes } = buildConfig({ | ||
| confirmations: BigInt(0), | ||
| requiredDVNs: [], | ||
| optionalDVNs: [], | ||
| optionalDVNThreshold: 0, | ||
| }) | ||
| expect(decodeRequiredDVNCount(ulnConfigBytes)).to.equal(255) | ||
| }) | ||
|
|
||
| it('encodes a concrete requiredDVNs by its length', () => { | ||
| const { ulnConfigBytes } = buildConfig({ | ||
| confirmations: BigInt(0), | ||
| requiredDVNs: [DVN], | ||
| optionalDVNs: [], | ||
| optionalDVNThreshold: 0, | ||
| }) | ||
| expect(decodeRequiredDVNCount(ulnConfigBytes)).to.equal(1) | ||
| }) | ||
| }) |
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.
Uh oh!
There was an error while loading. Please reload this page.