Fix/ephemeral balance undelegation 2.0 #77
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…em program. Added new version of close_ephemeral_balance to support it. kept old one for backward-compatability
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
12 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
GabrielePicco
requested changes
May 8, 2025
There was a problem hiding this comment.
I believe the current approach is not ideal, as the discriminator is only set on delegation (which defeat the idea of using it as a filter to get all escrow accounts), plus the handling of the callback becomes hacky. I prosed in this PR an implementation that it's simpler by removing the discriminator: #78
* fix: remove discriminator * chore: simplify fix * fix: delegation record with system program as owner & add tests
GabrielePicco
approved these changes
May 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
What problem are you trying to solve?
Solution
How did you solve the problem?
Before & After Screenshots
Insert screenshots of example code output
BEFORE:
[insert screenshot here]
AFTER:
[insert screenshot here]
Other changes (e.g. bug fixes, small refactors)
Deploy Notes
Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.
New scripts:
script: script detailsNew dependencies:
dependency: dependency detailsGreptile Summary
Here's my concise review of the ephemeral balance undelegation changes:
Adds external undelegation functionality and improves ephemeral balance account management in the delegation program.
ExternalUndelegateinstruction (discriminator 15) insrc/discriminator.rsto handle external undelegation of accounts owned by DLPclose_ephemeral_balance.rsto use direct system program transfers instead ofclose_pdahelper, with proper ownership verificationEphemeralBalancestruct (discriminator 104) insrc/state/ephemeral_balance.rsas a marker type for temporary balance accountstest_top_up.rsverifying ownership transfers and account closure during undelegationto_bytesa const function and adding proper initialization checksThe changes appear well-structured and properly integrated with existing functionality, with good test coverage for the new features.