-
Notifications
You must be signed in to change notification settings - Fork 593
fix(txe): use empty scopes for zero address and add ContractSyncService tests #20425
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
Conversation
| const effectiveScopes = from.isZero() ? undefined : [from]; | ||
| // When `from` is the zero address (e.g. when deploying a new account contract), we return an | ||
| // empty scope list which acts as deny-all: no notes are visible and no keys are accessible. | ||
| const effectiveScopes = from.isZero() ? [] : [from]; |
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.
We are now making it consistent with the PXE
benesjan
left a comment
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.
Looks good!
Just note that when testing you want to test external behavior and not irrelevant implementation details. AI sometimes over-tests and writes superfluous tests testing implementation details. So it's good to clear these useless test cases.
yarn-project/pxe/src/contract_sync/contract_sync_service.test.ts
Outdated
Show resolved
Hide resolved
…ce tests ## Summary - **TXE scope fix**: When `from` is the zero address, TXE now returns `[]` (empty scopes, deny-all) instead of `undefined` (allow-all). This matches the behavior of `BaseWallet.scopesFor()` in the PXE/wallet path, where the zero address produces an empty scope list that causes `ensureContractSynced` to skip sync entirely. - **ContractSyncService tests**: Adds 19 unit tests covering scope-aware caching, cache invalidation (wipe/discard), overrides, concurrency, and error handling. Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
b11af16 to
caca1bb
Compare
…ce tests (#20425) ## Summary - **TXE scope fix**: When `from` is the zero address, TXE now returns `[]` (empty scopes, deny-all) instead of `undefined` (allow-all). This matches the behavior of `BaseWallet.scopesFor()` in the PXE/wallet path, where the zero address produces an empty scope list that causes `ensureContractSynced` to skip sync entirely. - **ContractSyncService tests**: Adds 19 unit tests covering scope-aware caching, cache invalidation (wipe/discard), overrides, concurrency, and error handling.
Summary
fromis the zero address, TXE now returns[](empty scopes, deny-all) instead ofundefined(allow-all). This matches the behavior ofBaseWallet.scopesFor()in the PXE/wallet path, where the zero address produces an empty scope list that causesensureContractSyncedto skip sync entirely.