fix: stop reporting outcomes the tooling did not observe - #22
Merged
Conversation
Ten places where a script answered a question it had not asked, or reported success it had not established. They share one shape: a call that fails, or returns nothing, is read as a fact about the chain rather than as a failed read. The allowlist check is the sharpest. `checkAllowList` is a no-op while the allowlist is disabled, so it returns without reverting for every address, and reading that as "permitted" answered yes for anybody, 0x0 included, on the one command an operator runs to confirm a restriction is in force. It now reads the enforcement state first and says which of the three situations it found. The allowlist listing had the same ambiguity: an empty list means every sender is permitted when the hooks were deployed without one, and nobody may send when an enforced list was emptied. The listing now names the enforcement state outright, so an enforced empty list reads as what it is instead of being inferred from the absence of a warning. The pool deploys resolve decimals from the token when decimals() answers and from an explicit DECIMALS otherwise; decimals() is optional in ERC20 and the pool takes the value as a constructor argument by design, treating an on-chain read as a cross-check only. What changed is that the value is never guessed: with both sources present they must agree (the pool constructor's own InvalidDecimalArgs check, surfaced before the broadcast with a message that names the fix), and with neither the deploy stops instead of substituting something. The number is immutable and scales every amount the pool moves, so a wrong one is wrong in a way nothing downstream can detect. The transfer preflight simulated the receiver as the sender whenever ORIGINAL_SENDER was unset, which is the field the source pool gates on, so a clean GO could be followed by a live SenderNotAllowed. The sender it simulated is now printed with the inputs and named in the verdict. The rest: an allowlist update with neither list set broadcast an empty change and reported success; adopting a token that supplied no usable symbol stored it under the literal "unknown", where the next such token overwrote it, and the TOKEN_SYMBOL fallback covers a symbol() answering the empty string the same as one that reverts, since telling an operator to set a variable and then ignoring it is the same defect one layer down; the pool version resolver told operators their pool was not a pool when the read had several possible causes, and now separates that from the case where nothing is deployed at the address, which is definite; the roles audit accepted anything with code as a Safe, so a declared threshold of zero reconciled clean against a contract that answers no Safe getter; a lockbox whose token could not be read printed nothing at all; and the finality read printed an error and exited zero, telling any wrapper checking the exit code that the read had succeeded.
|
👋 aelmanaa, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
SyedAsadKazmi
approved these changes
Aug 4, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This pull request improves the clarity, reliability, and safety of deployment and diagnostic scripts for token pools, with a particular focus on how token decimals and symbols are resolved, allowlist enforcement is reported, and diagnostics are presented. The changes add more robust error handling, stricter checks, and clearer user guidance in both deployment and operational scripts.
Deployment and Pool Configuration Improvements:
decimals()and an explicitDECIMALSvalue (when both are provided) agree; otherwise, deployment fails. The fallback logic is centralized inDeploymentUtils._resolveTokenDecimals, and all scripts and docs are updated to reflect this behavior. [1] [2] [3] [4] [5] [6]TOKEN_SYMBOL, preventing ambiguous or colliding registry entries.Allowlist Diagnostics and Enforcement:
Diagnostics and User Guidance:
ORIGINAL_SENDERexplicitly, ensuring the simulation matches the actual sender and preventing false positives. [1] [2] [3] [4]Codebase Maintenance:
Let me know if you want to discuss any of these changes in detail or need help understanding how they affect your workflow!