refactor!: move contract instance and container helper utilities to chain.contracts #898
Conversation
antazoey
force-pushed
the
refactor/rename-contract-method
branch
from
d9a50bb to
8e13858
Compare
antazoey
changed the title
src/ape/api/accounts.py
Outdated
| ) | ||
| self.chain_manager.contracts[contract_instance.address] = contract_instance.contract_type | ||
| return contract_instance | ||
| # NOTE: Type ignore because we know it is a contract instance at this point |
There was a problem hiding this comment.
Could throw a assert contract_instance in here to satisfy mypy
There was a problem hiding this comment.
mypy is mad because the type is BaseAddress and not ContractInstance
Normally I would check the type to satisfy mypy (or assert the type), but importing from the ape.contracts namespace is challenging (circular imports)
There was a problem hiding this comment.
how about assert not isinstance(contract_instance, AddressType)?
There was a problem hiding this comment.
It would be BaseAddress, but let me try
There was a problem hiding this comment.
Oh no that won't work because ContractInstance is a subclass of BaseAddress
There was a problem hiding this comment.
TBH I kind of wish instance_at() always returned a ContractInstance. I don't remember why we made it return Address when it does not find the contract
There was a problem hiding this comment.
TBH I kind of wish
instance_at()always returned aContractInstance. I don't remember why we made it returnAddresswhen it does not find the contract
I don't either. Let's change that? Or have it return Optional[ContractInstance] maybe
There was a problem hiding this comment.
Or maybe ape.Contract is more of the API to access the contract cache?
There was a problem hiding this comment.
ape.Contract calls instance_at() at under-the-hood and manager-access mixin impls use chain_manager.instance_at directly since they can't really import from the top-level namespace.
There was a problem hiding this comment.
See all the changes in the latest commit! Making this change makes things way easier and still fixes #777, as now things will fail loudly and cause alert.
What I did
fixes: #777
this was a breaking change we wanted to do since
create_contractis a confusing name.How I did it
removed method entirely and use methods in
chain.contractsHow to verify it
name good or nah?
Checklist