Skip to content

add contract errors for SACs #1559

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add contract errors for SACs #1559

wants to merge 2 commits into from

Conversation

ElliotFriend
Copy link
Contributor

@ElliotFriend ElliotFriend commented May 28, 2025

I've copied the contract errors from the built-in contracts crate.

Questions for specific people:

@briwylde08 Do the notes make sense? I'm sure there are ways they could be reworded or cleaned up, especially the ones that are like "never used i think" lol. Does this chunk of text make sense on the page? In the specific place I have it? Or, should we move it?

@dmkozh I searched in that repo for occurrences of ContractError::ErrorNameHere and tried my best to make sense of what I was seeing. Did I capture correctly what each one is doing? What's the best way to handle the "never used" errors?

refs: #1499

@ElliotFriend ElliotFriend requested review from dmkozh and briwylde08 May 28, 2025 15:52
@stellar-jenkins
Copy link

@briwylde08
Copy link
Contributor

briwylde08 commented May 28, 2025

Tweaked wording a bit. Is there no additional context for error 13?

@stellar-jenkins
Copy link

@ElliotFriend
Copy link
Contributor Author

Is there no additional context for error 13?

The only place I found this error being thrown is in the SAC's balance.rs file. The function read_trustline_entry(...) similarly appears to be used only in that file. Mostly appears to do with balance operations (duh lol)? The functions that make use of the read_trustline_entry function are named get_trustline_balance, transfer_trustline_balance, get_trustline_flags, and set_trustline_authorization.

Not sure of a succinct way to incorporate that into the description, though.

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.

3 participants