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

fix/Support of non-corresponding instruction and context names #130

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

Ikrk
Copy link
Contributor

@Ikrk Ikrk commented Feb 27, 2024

This PR aims to fix a problem during the fuzz test generation in case the instruction and its corresponding context have different names, such as:

pub fn withdraw_unlocked(ctx: Context<Withdraw>) -> Result<()> {
       // ...
}

The correct context name was not taken into account and the fuzz test broke.

Now also the situation when one context struct is reused by multiple instructions is handled properly using a type alias. For example having another instruction re-using the context Withdraw above:

 pub fn withdraw_dummy(ctx: Context<Withdraw>) -> Result<()> {
        // ...
}

will generate a type alias and the deserialization will be reused:

pub type WithdrawDummySnapshot<'info> = WithdrawUnlockedSnapshot<'info>;

@Ikrk Ikrk force-pushed the fix/non-corresponding-ix-and-ctx-names branch from c8bfc37 to 74c534c Compare February 28, 2024 00:00
@Ikrk Ikrk marked this pull request as ready for review February 28, 2024 07:58
@Ikrk Ikrk requested a review from lukacan February 28, 2024 07:58
Copy link
Collaborator

@lukacan lukacan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! I have only one suggestion.

} else {
return Err(format!("The Context struct {} was not found", ctx_ident));
}
unique_ctxs.insert(ctx.clone(), ix_name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about, instead of storing ix_name, storing the already derived Snapshot name? I find it more intuitive to have pairs like (Context, SnapshotName).

@Ikrk Ikrk requested a review from lukacan February 28, 2024 16:47
Copy link
Collaborator

@lukacan lukacan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@lukacan lukacan merged commit 55ab963 into develop Feb 28, 2024
7 checks passed
@lukacan lukacan deleted the fix/non-corresponding-ix-and-ctx-names branch February 28, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants