-
Notifications
You must be signed in to change notification settings - Fork 11
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
IFeeModel/Subscription contract should be provided as a contract address #342
IFeeModel/Subscription contract should be provided as a contract address #342
Conversation
# if using a subcription, duration needs to be calculated | ||
if fee_model == "StandardSubscription": | ||
# if using a subscription, duration needs to be calculated | ||
if fee_model_contract.contract_type.name == "StandardSubscription": |
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.
Are you sure this will work? My understanding is that in L114, ape will instantiate fee_model_contract
as a generic Contract
, so I'm unsure that fee_model_contract.contract_type.name
will be available for this check.
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.
Yep. Tested it in ape-console
and by running the command using Marlin's subscription contract and then quitting once the time came to sign a tx. You can try it out.
Using Contract(...)
should automatically fetch the information from the ape-etherscan
configured explorer, if it can't get it locally - https://github.com/ApeWorX/ape/blob/v0.8.14/src/ape/managers/chain.py#L1339. It can obtain the ABI from the explorer.
This has been further improved in newer Ape versions as well - https://docs.apeworx.io/ape/stable/userguides/contracts.html#from-any-address (not our current version, but shows that the pattern holds moving forward).
…sses instead of by name. Subscription contracts are different for each adopter.
c8728f9
to
1983a67
Compare
Type of PR:
Required reviews:
What this does:
SubscriptionContracts/Fee Models are per adopter i.e. the address used is different every time. Therefore, parameters that take this value should take the value as a
Issues fixed/closed:
Why it's needed:
Notes for reviewers: