docs: add missing storage interface declarations#799
Open
Conversation
Contributor
Thanks for pointing this. I understand the same happens for other interfaces that already support the interfaceId, but should not be an error for integrators. Our methods that check the interfaceId are already compiled.
I think that could not be fully true, from a hook, you could read those values != 0 inside of a transaction.
Also good point. Not sure if adding an inheritance in the contract to a possible new interface modifies the bytecode, maybe not, and this can be added. Although this contract will never be used directly by externals |
Contributor
|
To avoid the Tag Env CI issue, I think we just need to set in the pharos env the commit |
|
Coverage after merging docs_add-storage-interface-abi into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Product requirements
src/were missing declarations for public state variable getters and explicit view/pure functions that exist on the implementing contractsDesign notes
IBatchRequestManagerchanged its ERC-165 interface ID from0xaf00afbdto0x5cdb8e3cIRoot,IHub,IGateway).Intentionally excluded (reviewers: speak up if you disagree)
BATCH_LOCATORS_SLOT(),GLOBAL_POOL()(x3),MAINNET_CENTRIFUGE_ID(),BASE_COST(),RECOVERY_TOKEN_EXTRA_COST(),DEFAULT_SUPPORTED_TX_LIMIT(),RECEIVE_COST()(x4),FREEZE_BIT(). These are internal tuning knobs and sentinel values that integrators should not depend on. They can change between versions without notice and exposing them in interfaces suggests they are part of the contract's public API.debited()andcredited()onIAccounting. These areuint128 public transientvariables that always return 0 when read externally (transient storage is cleared after each transaction). Declaring them in the interface would mislead integrators into thinking they carry meaningful state.IVaultFactoryis shared acrossAsyncVaultFactoryandSyncDepositVaultFactory. Factory-specific public variables likeasyncRequestManager()orsyncDepositManager()were not added to avoid breaking the shared interface.