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.

Sign up for GitHub

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

Jump to bottom

Clean up burn command #3976

Merged
merged 19 commits into from
Oct 3, 2024
Merged

Clean up burn command #3976

merged 19 commits into from
Oct 3, 2024

Conversation

casey
Copy link
Collaborator

@casey casey commented Oct 3, 2024

No description provided.

@casey
Use recipient instead of receiver
36a3960 
We already use `recipient` in the rest of our codebase
@casey
Use if let Some(recipient) instead of unwrap
7ee326d
@casey
Remove unnecessary pub
21ca2bf 
TARGET_POSTAGE is the default, so we don't need to specify it.
@casey
recipient_as_address() → recipient_address()
d55ddf3 
It's shorter and just as clear.
@casey
Use correct value name
b5cf135
@casey
Import Outgoing globally
b4cb268
@casey
Replace MAX_BURN_SATS with TARGET_POSTAGE
008cbb0 
TARGET_POSTAGE is already "not too many sats", so semnatically it's the
same as MAX_BURN_SATS, and both are 10,000 sats.
@casey
Improve error handling in burn
64450ba
@casey
Remove runes_cannot_be_burned test
f7d4baa 
Tests should be written to cover bugs that we might conceivably have.
However, in this case, the argument to the burn function is an
inscription ID, so we cannot conceivably accept a rune.
@casey
inscription_id -> inscription
d46fe1a 
We already know it's an inscription ID from the type
@casey
Avoid if let
5670313
@casey
burn_script -> script_pubkey
cc937d2
@casey
Improve some test names
a4776f7
@casey
inscribe_with_custom_postage -> inscribe_with_postage
d1bf6d2
@casey
Preserve invalid address source error
2ed4bc6
@casey
Clean up dust checking
245101a
@casey
Use slice instead of vec
b0d6c41
@casey
Clone instead of to_vec
21f3934
@casey casey requested a review from raphjaph October 3, 2024 19:55
raphjaph
raphjaph approved these changes Oct 3, 2024
View reviewed changes
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

LGTM!

@casey casey merged commit 46611bd into ordinals:master Oct 3, 2024
5 checks passed
@casey casey deleted the refactor-burn branch October 3, 2024 20:57
@BrewTestBot BrewTestBot mentioned this pull request Oct 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphjaph raphjaph raphjaph approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@casey @raphjaph