-
Notifications
You must be signed in to change notification settings - Fork 51
Add support for custom action #483
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
state/src/item/cache.rs
Outdated
| impl CacheableItem for Bytes { | ||
| type Address = H256; | ||
| fn is_null(&self) -> bool { | ||
| self.len() == 0 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
codechain/config/chain_type.rs
Outdated
|
|
||
| impl ChainType { | ||
| pub fn spec<'a>(&self) -> Result<Spec, String> { | ||
| pub fn spec<'a>(&self, handlers: &[Arc<ActionHandler>]) -> Result<Spec, String> { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
core/src/spec/spec.rs
Outdated
|
|
||
| /// Load from JSON object. | ||
| fn load_from(s: cjson::spec::Spec) -> Result<Spec, Error> { | ||
| fn load_from(s: cjson::spec::Spec, handlers: &[Arc<ActionHandler>]) -> Result<Spec, Error> { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| Ok(()) | ||
| } | ||
|
|
||
| fn is_target(&self, bytes: &Bytes) -> bool { |
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.
How about remove this method
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.
We need some sort of function that only checks if provided byte array is valid one that this handler can decode, without execution. It's used for blocking invalid parcels in sync and rpc.
Since execute requires TopLevelState as argument, I think it's not a good idea to merge these.
state/src/action_handler/hit.rs
Outdated
| } | ||
|
|
||
| /// `bytes` must be valid encoding of HitAction | ||
| fn execute(&self, bytes: &Bytes, state: &mut TopLevelState) -> StateResult<Outcome> { |
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.
and make this function return Option<StateResult<Outcome>>?
| }) | ||
| } | ||
| Action::Custom(bytes) => { | ||
| let handlers = self.db.custom_handlers().to_vec(); |
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.
Why do you convert it do Vec?
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.
custom_handlers returns reference, so handlers borrows self.
self is used as mutable reference in later code, so lifetime conflicts there.
So I wanted to clone handlers, and I thought to_vec would be a good solution.
| }) | ||
| } | ||
|
|
||
| fn custom_handlers(&self) -> Vec<Arc<ActionHandler>> { |
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.
Why noy fn custom_handlers(&self) -> &[Arc<ActionHandler>>] {
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.
If we return reference, then lock over state_db won't be released while holding that reference.
I thought it would be simpler to return a fresh copy of the vector and release the lock earlier instead of holding the lock.
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.
Okay. I see.
952e821 to
33008a3
Compare
No description provided.