refactor(contracts)!: StreamManager upgrades #112
Conversation
- `Stream.stream_id -> Stream.id` - reordering/renaming of some event arguments - reordering of `create_stream` function - remove unused functions
bigger refactor that computes the streaming rate as the sum of product validations, instead of requiring an explicit streaming rate an intent parameter `min_stream_life` can be used to control for any changes from what the expected stream life may be (but can always be more)
|
ca8c85b related: ApeWorX/ape#2277 |
|
something weird I noticed, can't tell if it because of Ape or the ApePay library, but if you run the tests w/o contracts being compiled, it will compile them, but then all tests will fail with Edit: ok this hasn't happened in my sample project, hmmm |
There was a problem hiding this comment.
I did a quick pass. Want to come back and do another a little later.
suggestion: document more. Maybe file-level docstrings in the header, or maybe in the README.
question: What exactly is a product? Are there any constraints? If it's just up to validators what exactly that means to the contract, maybe that should be explained a bit more.
Looking at the demo it leaves me a bit confused:
>>> HashBytes32(b"\x00" * 24 + b"\x01" + b"\x00" * 7)
HexBytes('0x0000000000000000000000000000000000000000000000000100000000000000')
>>> 0x0000000000000000000000000000000000000000000000000100000000000000
72057594037927936
>>> 0x0000000000000000000000000000000000000000000000000100000000000000 // 1e18
0.0Even if you adjust for different decimals for a token, I'm not sure how this matches the comment of "256 tokens/hour". Feel like I'm missing something.
| BLUEPRINT, | ||
| msg.sender, # Only caller can create | ||
| ONE_HOUR, # Safety parameter (not configurable) | ||
| min_stream_time, # Safety parameter for new streams | ||
| validators, | ||
| accepted_tokens, # whatever caller wants to accept | ||
| salt=convert(msg.sender, bytes32), # Ensures unique deployment per caller |
There was a problem hiding this comment.
question: I know it isn't being changed here, but out of curiosity, why limit one stream manager per sender?
There was a problem hiding this comment.
originally I was thinking that it would make sense to keep it like that and have per-product validators, but I think what I am gonna do is add a secondary String[32] "product" variable that also gets concatenated to the salt (and to the deployments mapping)
this way, in the StreamFactory Python SDK class I can actually loop through all of the known deployments of the factory (which I want to use the release version identifier as the create2 salt so you can get release by address) and look for the deployments that a particular user has made for a particular product name
Yes, it is up to the validator. 👍 on better docs/explanation there (but right now it's more for us)
The "TestValidator" in there basically just converts the product code directly into the streaming rate, so that hex number is Our product's pricing algorithm is obviously going to be a lot more specific to how the Silverback product code is structured. Multiple codes are allowed if people want to merge streams together (pay for multiple clusters from the same stream, but not shipping out of the box w/ that). It's a lot easier to upgrade validators than it will be to upgrade the StreamManager, so the logic there is more open since it can change (e.g. as we update our pricing structure all new streams will have to use it, need some code updates to apply to existing streams as well)
Don't forget that the token decimals are really only for display purposes only >>> 0x0000000000000000000000000000000000000000000000000100000000000000 / 1e18
0.07205759403792794 |
|
note: coverage/gas reporting seems to be failing erronously |
Co-authored-by: antazoey <[email protected]>
also add `@dev` to Validator.vyi
|
Deployed a preliminary cut here (using salt Our deployment |
| def is_accepted(self) -> ContractCallHandler: | ||
| return self.contract.token_is_accepted | ||
| def is_accepted(self, token: AddressType) -> bool: | ||
| return self.contract.token_is_accepted(token) |
There was a problem hiding this comment.
We could potentially use the ape-extras mechanism and feed into .contract so all pass throughs are automatic.
There was a problem hiding this comment.
How does that work again?
Co-authored-by: antazoey <[email protected]>
What I did
Stream.stream_id -> Stream.idcreate_streamfunctionfixes: #103
fixes: #104
fixes: #105
fixes: #107
partial fix: #72
How I did it
some larger refactors to contracts, examples, py sdk, etc.
NOTE: No update to js-sdk
How to verify it
Checklist