Skip to content
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

Oracles: Correct feeder encoding #1793

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Oracles: Correct feeder encoding #1793

merged 2 commits into from
Apr 3, 2024

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Apr 3, 2024

Description

This is not a current issue because the Feeder wrapper type does not add any byte to its serialization/deserialization, but it is a potential one if this type changes.

@lemunozm lemunozm added Q0-trivial An issure which is similar to patching code. P4-required Issue should be addressed labels Apr 3, 2024
@lemunozm lemunozm self-assigned this Apr 3, 2024
@lemunozm
Copy link
Contributor Author

lemunozm commented Apr 3, 2024

I've also been scared because clippy for rust-1.78 complains about the Feeder eq implementation:

warning: function cannot return without recursing
  --> runtime/common/src/oracle.rs:38:2
   |
38 | /     fn eq(&self, other: &Self) -> bool {
39 | |         self.0.eq(&other.0)
40 | |     }
   | |_____^
   |
note: recursive call site
  --> runtime/common/src/oracle.rs:39:3
   |
39 |         self.0.eq(&other.0)
   |         ^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unconditional_recursion
   = note: `#[warn(clippy::unconditional_recursion)]` on by default

Anyway, it seems to be well defined, and I tested with the following and it pass:

#[cfg(test)]
mod tests {
use super::*;
    #[test]
    fn foo() {
        let a = Feeder::<RuntimeOrigin>::root();
        let b = Feeder::<RuntimeOrigin>::none();
        assert_ne!(a, b);
        assert_eq!(a, a);
        assert!(!(a < a));
        assert!(a <= a);
    }
}

@lemunozm lemunozm enabled auto-merge (squash) April 3, 2024 08:59
@lemunozm
Copy link
Contributor Author

lemunozm commented Apr 3, 2024

Regarding the above clippy warning. It seems a clippy issue: rust-lang/rust-clippy#12245

@lemunozm lemunozm merged commit a35def2 into main Apr 3, 2024
10 checks passed
@lemunozm lemunozm deleted the fix/feeder branch April 3, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4-required Issue should be addressed Q0-trivial An issure which is similar to patching code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants