Conversation
…rdless of block number)
|
bot fmt |
|
@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3278607 was started for your command Comment |
|
@kianenigma Command |
ggwpez
left a comment
There was a problem hiding this comment.
Yea looks good, just some nits.
| //! interact with an offchain worker asynchronously. It also showcases the | ||
| //! potential pitfalls and security considerations that come with it. | ||
| //! | ||
| //! It is based on [this example by |
There was a problem hiding this comment.
Do you know by any chance what license this was originally published under?
There was a problem hiding this comment.
The original offchain-worker example pallet was published under Apache-2.0:
|
|
||
| /// A friendlier name for the error that is going to be returned in | ||
| /// case we are in the grace period. | ||
| const RECENTLY_SENT: () = (); |
There was a problem hiding this comment.
| const RECENTLY_SENT: () = (); |
I dont think this adds much.
There was a problem hiding this comment.
I do understand the author's original intent to have a descriptive error. So in the places where it is used, replace it with an empty tuple?
substrate/frame/examples/offchain-worker-price-oracle/src/lib.rs
Lines 257 to 258 in a08af90
There was a problem hiding this comment.
Also want to note that this came from the original implementation:
Please let me know how you'd like me to move forward.
There was a problem hiding this comment.
Please return () as error without any alias, it is not a good rust pattern.
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
| let val = StorageValueRef::persistent(b"example_ocw::last_send"); | ||
| // The Local Storage is persisted and shared between runs of the | ||
| // offchain workers, and offchain workers may run concurrently. We | ||
| // can use the `mutate` function, to write a storage entry in an |
There was a problem hiding this comment.
The whole line wrapping here seems 80 rather than 100 🙄
There was a problem hiding this comment.
@kianenigma it seems that your comment is pointing to the code before this commit, which addresses line wrapping
here's the same snippet after the commit:
substrate/frame/examples/offchain-worker-price-oracle/src/lib.rs
Lines 230 to 236 in 5ec0432
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
The CI pipeline was cancelled due to failure one of the required jobs. |
As discussed in:
this PR is refactoring FRAME's Offchain Worker examples in order to mitigate common misunderstandings that we've been seeing in the community (which can result in security issues)
the original OCW example was actually split into two separate examples:
Ping Pong
inspired by https://gnunicorn.github.io/substrate-offchain-cb/
this example attempts to illustrate a scenario where some user input triggers some action by the OCWs... in this case, these are simple
ping-pongevents, where thepongcan happen as many kinds of transactions, namely:the comments and docs put a strong emphasis into the security issues that come from using unsigned transactions, and the user is encouraged to use signed transactions from authorized/permissioned accounts instead.
Price Oracle
follows the same spirit from the original example (a
BTC/USDprice oracle), but new prices can only be submitted by authorized/permissioned accounts, which removes the possibilty of malicious actors submitting false prices via unsigned transactions.The main purpose of this example is to:
cryptocompare's API in this case)