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

smol fix for process_remove_member.rs #1166

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

staccDOTsol
Copy link
Collaborator

plz sers, I will earn so much of my hydra wasted efforts back if we can fix the remove Member ix.

expected behavior: feed it a member (which is seeded as mint of membership voucher) and a destination (native account) and it works
actual behavior: it does a check to see if member is owned by spl token, errors if so with "InvalidCloseAccountDestination. Error Number: 6024. Error Message: Sending Sol to a SPL token destination will render the sol unusable."
fix: this check is meant for the destination, to see if it is a native account. do a switcheroo and the checks should pass.

plz sers, I will earn so much of my hydra wasted efforts back if we can fix the remove Member ix.

expected behavior: feed it a member (which is seeded as mint of membership voucher) and a destination (native account) and it works
actual behavior: it does a check to see if member is owned by spl token, errors if so with "InvalidCloseAccountDestination. Error Number: 6024. Error Message: Sending Sol to a SPL token destination will render the sol unusable."
fix: this check is meant for the destination, to see if it is a native account. do a switcheroo and the checks should pass.
it's actually better like this, tbf.
@staccDOTsol
Copy link
Collaborator Author

edits, yes I'm doon this in the browser:

check if destination not owned by system program, if not, error with more appropriate error.

@staccDOTsol
Copy link
Collaborator Author

hi :) @blockiosaurus

@staccDOTsol
Copy link
Collaborator Author

I'll personally be at least 42 sol up when this gets merged

🙏🏻

@blockiosaurus
Copy link
Contributor

Can you fix the formatting so the CI passes?

Copy link
Contributor

@blockiosaurus blockiosaurus left a comment

Choose a reason for hiding this comment

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

The CI just needs to pass and this is good to go.

@staccDOTsol
Copy link
Collaborator Author

roger, wip

@staccDOTsol
Copy link
Collaborator Author

alright @blockiosaurus I have no idea how to see which ci are failing or why
I tried brew install act act push and ehhh it refuses to run the cicd for hydra cuz of it doesn't know the default branch
can you highlight or link the particular cicd report

@staccDOTsol staccDOTsol requested a review from a team as a code owner August 24, 2024 22:06
@staccDOTsol staccDOTsol requested review from shotgunofdeath and danenbm and removed request for a team August 24, 2024 22:06
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