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

refactor(executor): Check custom visit functions in #[derive(Visit)] #4632

Merged

Conversation

dima74
Copy link
Contributor

@dima74 dima74 commented May 21, 2024

Description

While working on #4599, I noticed that in custom executor it is possible to derive Visit with invalid #[visit(custom())] functions which will be ignored (only rustc error that function is unused). This PR adds check for custom visit function names to avoid potential bugs. Also I updated documentation of some proc-macros related to custom executor.

#[derive(Constructor, ValidateEntrypoints, Validate, Visit)]
#[visit(custom(visit_register_domain2))]  // typo in visit_register_domain
struct Executor {
    verdict: Result,
    block_height: u64,
}
fn visit_register_domain(...) { ... }

Linked issue

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@dima74 dima74 force-pushed the diralik/executor-derive-Visit-check-custom branch from cbb84b2 to 0c8d42d Compare May 22, 2024 08:16
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
@mversic mversic force-pushed the diralik/executor-derive-Visit-check-custom branch from 0c8d42d to 90bea9e Compare May 22, 2024 08:28
@mversic mversic merged commit 5b591db into hyperledger:main May 22, 2024
12 of 13 checks passed
@dima74 dima74 deleted the diralik/executor-derive-Visit-check-custom branch May 22, 2024 08:36
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.

None yet

4 participants