Skip to content

chore: improved error logs + docs and naming #69

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

Merged
merged 28 commits into from
Mar 8, 2025
Merged

Conversation

thlorenz
Copy link
Contributor

@thlorenz thlorenz commented Mar 7, 2025

Summary

Improving error logs, documenting each instruction in more detail and normalizing the protocol
fees vault related instruction name.

Details

Error Logs

When an error occurred we would end up with just an error code and it was hard to understand
what went wrong. We now log error messages to include more information about the error
and the context in which it occurred, mainly a label and pubkey of the account in violation.

Documentation

Each instruction now has the following sections in the documentation:

  • accounts: a list of accounts that are used by the instruction and if they need to be
    writable/signers
  • requirements: a list of requirements that need to be met for the instruction to succeed,
    mainly relating to how the input accounts were setup previously
  • steps: outlining what the instruction does (those were already there for most instructions,
    and were added to all remaining ones)

Greptile Summary

This PR enhances documentation and error logging across the delegation program, focusing on improved clarity and maintainability.

  • Added comprehensive documentation sections for accounts, requirements, and execution steps across all processor functions
  • Renamed InitFeesVault to InitProtocolFeesVault and related functions for better clarity
  • Added descriptive labels to account validation functions for more informative error messages
  • Fixed error enum name from Undelegatable to NotUndelegatable for clearer error messaging
  • Added TODO comments flagging potential issues like obsolete fields and validation requirements

The changes are well-structured and improve code maintainability without introducing functional changes. However, the TODO comments should be addressed before merging.

@thlorenz thlorenz requested a review from GabrielePicco March 7, 2025 11:07
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

37 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings | Greptile

thlorenz and others added 5 commits March 7, 2025 04:13
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Contributor

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@GabrielePicco GabrielePicco merged commit d582a00 into main Mar 8, 2025
3 checks passed
@GabrielePicco GabrielePicco deleted the thlorenz/docs+logs branch March 8, 2025 17:21
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