-
Notifications
You must be signed in to change notification settings - Fork 308
Implement accumulator updates for cosmwasm #880
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
jayantk
left a comment
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.
looks great!
| to_price_feed(&mut feed3).publish_time += 1; | ||
| to_price_feed(&mut feed1).price *= 2; | ||
| to_price_feed(&mut feed2).price *= 2; | ||
| to_price_feed(&mut feed3).price *= 2; |
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.
nitpick: with a name like to_price_feed, this doesn't sound like it should mutate the underlying data. that said, I can't think of a better name for it, so 🤷
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.
Tried to conform to this
| StdError::from(PythContractError::InvalidAccumulatorMessage) | ||
| ); | ||
| } | ||
|
|
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.
👍 nice & thorough tests
ali-behjati
left a comment
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.
Very nice!
I think it would be good to add a few more tests that i mentioned in an inline comment. Please add them before merging.
The fee for accumulator messages is base fee times the number of messages but the logic remains the same for the batch method
ali-behjati
left a comment
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.
🚢 it
No description provided.