-
Notifications
You must be signed in to change notification settings - Fork 0
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
[pr] Borsh Serialize solana_sdk::Transaction #8
Conversation
sdk/src/signature.rs
Outdated
} | ||
|
||
// Implement Arbitrary for Signature | ||
impl<'a> arbitrary::Arbitrary<'a> for Signature { |
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.
Just a Rust language question.. was the lifetime annotation required here? I would've thought this was a case where it was just implicit.
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 was required by trait definition of arbitrary::Arbitrary
pub trait Arbitrary<'a>: Sized {
Can you elaborate on why Arbitrary and Proptest are needed? It'd be helpful to link code snippets as well. |
They're testing libraries, so even if we do need them, I believe they should be gated behind configuration flags. |
I see some sovereign modules use Arbitrary and Proptest for fuzz testing. And in the module template we had those created derived, I mean they had the following in callMessage
I believe we will need those crates in the future for more testing to find bugs in the module. Therefore, decided not to remove it from CallMessage and implement it for the Transaction type. |
You are right, I missed adding flags here. I did not think about them and totally forgot. Will change that |
Problem
In svm-rollup pr we are using Transaction inside of CallMessage, which requires Borsh
Summary of Changes
Added
BroshSerialization/Deserialization
toTransaction
struct, and other structs inside of it, such asSignature
,CompiledInstruction
, etc.Fixes #