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.
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
feat(pulse): add pulse contracts #2090
base: main
Are you sure you want to change the base?
feat(pulse): add pulse contracts #2090
Changes from all commits
a7a391a
9633c25
eeeaaf5
8196f78
a11ccd3
43296d3
412434f
38962a1
040cedb
2a11e71
5977b51
f3a56dd
f60194a
ce1e3ec
0e5e328
6702d2f
938c216
f3ec247
13c3967
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
what's this?
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.
what is this for :?
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.
Hmm thinking about it, now that we don't emit the ids and don't store them (we store the hash of it), how argus is supposed to understand it :? parse the transactions?
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.
I recommend allocating an extra overhead for cross-contract calls.
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's good to emit something (for debugging purposes), just be aware that it might be expensive to do this in ethereum mainnet.
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.
I'm wondering how this works in Entropy. Do we update pythFee regularly based on gasPrice? @m30m might know better but similar products charge fee as % of the tx fee.
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.
There is also one DoS attack vector in our price feeds use case regarding gas prices. People can ask for price updates anytime in the future and it won't be fullfilled immediately (opposed to Entropy) and this might make
tx.gasprice
a worse estimate on normal usage (only a few seconds after that) or can be taken advantage of to make hundreds of requests in the future when the gas price is lower.My recommendation is to not allow a publish time that is more than 1min in the future. Or charge higher fees as it goes more into the future.