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

premine percentage and supply fields update for rune.html #3748

Merged
merged 24 commits into from
May 24, 2024

Conversation

cryptoni9n
Copy link
Collaborator

@cryptoni9n cryptoni9n commented May 13, 2024

presents options to fix #3630

  • this is my first attempt at making any sort of real changes using Rust. I am still in the very early stages of learning Rust and so I wanted to add a disclaimer that these changes were not done with much proficiency - there are probably smarter and better ways to have made them. I am eager to learn so please provide feedback on what I can do better. I don't think that all of these fields are necessary for the rune page, but I wanted to display them all so that they could be reviewed and a final decision as to which to keep can be made while considering all possibilities.

changes made:

  • added mint progress field
  • renamed supply to current supply
  • added maximum supply field
  • added supply progress field
  • renamed premine percentage topremine percentage of current supply
  • added premine percentage of maximum supply formula

presents options to fix ordinals#3630
@cryptoni9n cryptoni9n requested a review from raphjaph May 13, 2024 12:58
@HubertusVIE
Copy link

Thanks for this intiative! Do you think your'd be able to add #3677? It goes well with this one.

@cryptoni9n
Copy link
Collaborator Author

Thanks for this intiative! Do you think your'd be able to add #3677? It goes well with this one.

Hi HubertusVIE - the changes required for 3677 would be much, much more involved and I wouldn't even know where to begin. That change will need to be implemented by Casey and Raph because it involves upgrades to the protocol. This change that I've made only involves small updates to the rune webpage. They may seem like they go together nicely, but from a coding perspective, they are opposite ends of the spectrum.

cryptoni9n and others added 7 commits May 16, 2024 17:36
@cryptoni9n
Copy link
Collaborator Author

I've reverted these changes in order to better understand the test failures they're triggering. Still working on this, just need to reevaluate the best way to move forward. Renaming the supply field in the .html page causes over 90 tests to fail.

@casey
Copy link
Collaborator

casey commented May 20, 2024

I think doing these one at a time would be a good idea. So, for example, start with supply progress, which I would actually call mint progress in a single PR. It's usually a good idea to make PRs simple and focused, so they can be reviewed easily, and then follow up with additional small PRs.

adding new field `mint progress` to rune.html to show the current supply of max supply ratio.
update batch function to account for new `mint_progress` field
added mint_progress to runes_are_displayed_on_rune_page test
added `mint_progress` to display test
@cryptoni9n
Copy link
Collaborator Author

I think doing these one at a time would be a good idea. So, for example, start with supply progress, which I would actually call mint progress in a single PR. It's usually a good idea to make PRs simple and focused, so they can be reviewed easily, and then follow up with additional small PRs.

Thank you, Casey - I appreciate your guidance and patience. I've taken your advice and just added the mint progress field to begin with. Please let me know what you think and what the next best step would be.

@casey
Copy link
Collaborator

casey commented May 22, 2024

Nice! I think mint progress should be expressed as a percentage with two digits after the decimal point. So it would be something like format!("{}%", minted as f64 / total_supply as f64}.

changing mint progress to percentage
changing `mint progress` to percentage
changing `mint progress` to percentage
changing `mint progress` to percentage
reverting previous changes; this file was accidentally included in this change.
changing `mint progress` to percentage
@cryptoni9n
Copy link
Collaborator Author

Nice! I think mint progress should be expressed as a percentage with two digits after the decimal point. So it would be something like format!("{}%", minted as f64 / total_supply as f64}.

how's this look to you?

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.

Thanks for simplifying this PR, this is much easier to review!

LGTM

@raphjaph
Copy link
Collaborator

Another tip for follow-up PRs:

It's always best to work on feature branches. Assuming you're on your master branch, this is what I recommend:

First pull in the latest changes from the ord (origin) repo by doing:

git pull origin master

Then create a local branch:

git checkout -b show-current-supply

Then make you changes and do git add, git commit, etc. Try to do a git push. You'll probably be prompted to something like this:

 git push --set-upstream cryptoni9n show-current-supply

Open a PR from that local branch which would have the name cryptoni9n/show-current-supply. After the PR has been reviewed and successfully merged you can delete the local branch by doing.

git checkout master
git branch -D show-current-supply

Repeat this for every feature/PR and you'll have a good work flow :)

@raphjaph raphjaph merged commit aba1d7a into ordinals:master May 24, 2024
5 checks passed
@cryptoni9n
Copy link
Collaborator Author

Thanks for the tip, @raphjaph - I've been really struggling with figuring out that process and so that is immensely helpful for me. I was already able to use this process once successfully on a new PR I just submitted.

@casey thank you again for your help on this. what would you recommend for the first additional follow up PR? I feel like the Premine Percentage isn't very intuitive in many cases. I would like to see it change or replace it with a different metric expressing the percentage as (premine/max_supply). In my previous attempts, I had renamed the supply field to current supply and renamed the premine percentage field to premine percentage of current supply. I also added new fields for maximum supply and premine percentage of maximum supply but I'm not sure all of that is necessary.

I hope memeing is supported on this platform.

7bfb1cc0245d6aefa017517d0e7e29a3361805c9787afb7e52686a82473820b9

@casey
Copy link
Collaborator

casey commented May 25, 2024

You bet!

I'm not actually sure I would want to expose "premine percentage of maximum supply", because it might not correspond to anything meaningful. The maximum supply can be very high, but if it never actually mints out, then that percentage is a bit misleading.

@cryptoni9n
Copy link
Collaborator Author

You bet!

I'm not actually sure I would want to expose "premine percentage of maximum supply", because it might not correspond to anything meaningful. The maximum supply can be very high, but if it never actually mints out, then that percentage is a bit misleading.

I see that point and agree. I feel the heart of the issue is when people would go to this mintable example rune page and see that the premine percentage is 99.83%, it seems to convey two incorrect ideas: 1) there is less than 1% left to mint and, 2) the etcher premined almost 100% of the rune. I understand that both of these are misconceptions based off of an incorrect understanding of the metric, but a lot of people don't get it visually.

At the same time, going to an unmintable example rune page and seeing the premine percentage of 100% makes perfect sense because it isn't mintable anymore and the same assumptions above are now correct, 1) there is 0% left to mint and 2) the etcher premined 100% of the rune.

I'm sure this is over complicating things, but it's almost as if there should be different stats for mintable vs unmintable/expired mint runes. Mintable runes could show current and maximum possible supply stats and percentages, to showcase the growth capability. Unmintable could have percent premined and other hard locked statistics, (supply instead of current or max supply, etc.). Also, I feel like showing the terms for an unmintable rune is largely unnecessary and should be de-emphasized somehow.

I'm sorry I'm not presenting any great solution suggestions, but this is the crux of what I think needs to be addressed. If there are no good ideas right now, we can park it for awhile and I will continue to noodle it.

@casey
Copy link
Collaborator

casey commented May 26, 2024

I do think it would be a good idea, once a rune is unmintable, to display the premine percentage as a percentage of the final supply.

danadi7 added a commit to sadoprotocol/ord that referenced this pull request Jul 15, 2024
* Fix type in runes docs (ordinals#3447)

* Fix deploy bitcoin.conf typo (ordinals#3443)

* Remove `etch` from error message (ordinals#3449)

* Update rune docs for Chinese version (ordinals#3457)

* Update testing.md (ordinals#3463)

* Fix typo in zh.po (ordinals#3464)

* Update required Rust version in README (ordinals#3466)

* Add package necessary for Ubuntu (ordinals#3462)

* Add rune logo and link to navbar (ordinals#3442)

* Fix maturation loop (ordinals#3480)

* Add wallet batch outputs and inscriptions endpoints (ordinals#3456)

* Check etching commit confirmations correctly (ordinals#3507)

Previously, we were checking if an etching commit transaction had matured by
asking bitcoind how many confirmations it had. This was incorrect, because if
ord was behind bitcoind, the number of confirmations would be based on
bitcoind's current block height, and would be greater than the number of
confirmations the commit transaction had as of the reveal transaction block
height.

This commit fixes this by calculating the number of confirmations using the
block height of the commit transaction, which is correct even if bitcoind is
ahead.

* Show decimal rune balances (ordinals#3505)

* Allow inscribing without file (ordinals#3451)

This allows inscribing only the delegate without having to specify a file. In the batch it allows you to comletely omit the file as well.

* Add etching turbo flag (ordinals#3511)

Future runes features may be opt-in, for example, if they increase light client
validation costs, or if they are simply too degenerate.

This commit adds a "turbo" flag, which, if set, premptively opts in to such
future features.

It also sets the "turbo" flag on the genesis rune, UNCOMMON•GOODS.

Because the genesis rune is inserted into the index when it is created, the
schema version is also incremented, to force rebuilding indices created before
this commit.

* Update data carriersize to match with ord (ordinals#3506)

* Document allowed opcodes in runestones (ordinals#3461)

Document that data push opcodes 0 through 78 inclusive are allowed in
runestones, and opcodes 79 and above will produce a cenotaph. Also add a test
that makes sure that this is actually true.

* Fix typo in zh.po (ordinals#3498)

* Better error message when bitcoind doesn't start (ordinals#3500)

* Updated rust-version to 1.74.0 (ordinals#3492)

Ord now requires Rust 1.74 to build, update `rust-version` in Cargo.toml files
to reflect this.

* Mint with destination (ordinals#3497)

* Bump ord crate required rust version to 1.76 (ordinals#3512)

* Add postage flag to mint command (ordinals#3482)

* Test that mints without a cap are unmintable (ordinals#3495)

Test that if the `cap` field of an etching is unset, it is treated as being
zero, and the rune is unmintable.Test that if the `cap` field of an etching is
unset, it is treated as being zero, and the rune is unmintable.

* Release 0.18.0 (ordinals#3513)

- Bump version: 0.17.1 → 0.18.0
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Fix off-by-one in wallet when waiting for etching commitment to mature (ordinals#3515)

While waiting to broadcast a rune etching, take into account the fact that the
commit transaction will have an additional confirmation in the next block.
Previously we weren't, so the wallet was waiting for one confirmation too many.

* Release 0.18.1 (ordinals#3522)

- Bump version: 0.18.0 → 0.18.1
- Update changelog
- Update changelog contributor credits

* Forbid etching below rune activation height (ordinals#3523)

Don't allow etching if the etching reveal height is below the runes activation
height, otherwise the user would waste funds on an ineffective etching.

* Put rune higher on /inscription (ordinals#3363)

* Fix typo in recursion docs (ordinals#3529)

* Add Red Had build instructions to readme (ordinals#3531)

* Add runes pagination (ordinals#3215)

* Lookup rune by number (ordinals#3440)

* Emit rune-related events (ordinals#3219)

Emit rune etched, rune minted, rune transferred, and rune burned events to the
index event channel.

* Address runes review comments (ordinals#3547)

- Fix overflow in Rune::from_str
- Use checked_pow in Pile Display implementation
- Only report first flaw in cenotaph
- Add comment to Edict::from_integers for output == tx.output.len()
- Avoid divide by zero in RuneUpdater
- Add test for zero rune ID delta
- Make varint::decode return a Result instead of an Option

* Lock runes commit output (ordinals#3504)

* Display etched runes on /block (ordinals#3366)

* Remove /runes/balances page (ordinals#3555)

* Fix typo in zh.po (ordinals#3540)

* Fix typos (ordinals#3541)

* Store wallets in /wallets subdir of data dir (ordinals#3553)

Makes the data dir cleaner, and avoids a conflict with wallet named `index`,
since there's already an `index.redb`.

* Add command to export BIP-329 labels for wallet outputs (ordinals#3120)

Add `ord wallet label`, which exports BIP-320 wallet output labels. Especially
useful for labeling outputs in Sparrow wallets.

* Add open mint tests (ordinals#3558)

* Document turbo flag (ordinals#3579)

* Release 0.18.2 (ordinals#3582)

- Bump version: 0.18.1 → 0.18.2
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Bump ord version to 0.18.2 in Cargo.toml (ordinals#3583)

* Resume cycles through all pending etchings (ordinals#3566)

* Generate sample batch.yaml in env command (ordinals#3530)

* Add default content proxy and decompress to env command (ordinals#3509)

* Address runes review comments (ordinals#3605)

- Use inclusive range in Index::get_runes_in_block
- Remove out-of-date comment
- Use checked_pow in Decimal Display impl
- Use checked_pow in impl FromStr for Decimal

* Remove duplicated word (ordinals#3598)

* Show premine percentage (ordinals#3567)

* Add back runes balances API (ordinals#3571)

* Remove timeout for wallet client (ordinals#3621)

* Add `dry-run` flag to `resume` command (ordinals#3592)

* Clear etching when rune commitment is spent (ordinals#3618)

* Add test Rune cannot be minted less than limit amount (ordinals#3556)

* Update recursion.md with consistant syntax (ordinals#3585)

* Check rune minimum at height before sending (ordinals#3626)

* Release 0.18.3 (ordinals#3625)

* Update sparrow-wallet.md --name flag update (ordinals#3635)

* Fix zh.po translations (ordinals#3588)

* Allow minting if mint begins next block (ordinals#3659)

Make `ord wallet mint` check whether the rune to be minted is mintable in the
next block, since that's the block in which our mint transaction might actually
be mined.

* Do not show runic outputs in cardinals command (ordinals#3656)

* Update sat-hunting.md with how to transfer specific sats (ordinals#3666)

* Allow longer request body for JSON API (ordinals#3655)

* Clarify that inscriptions must be served from URLs with path /content/<INSCRIPTION_ID> (ordinals#3209)

In order for inscriptions to retrieve their own inscription ID, inscription
content must be served from URLs with path `/content/<INSCRIPTION_ID>`.

When an inscription with ID X delegates to another inscription with ID Y,
meaning that X has a delegate field whose value is Y, when requesting the
content of inscription ID X, the content of inscription Y must be returned from
the URL with path `/content/X`. This allows inscription Y to use the ID of
delegating inscriptions as seeds for generative content, since the delegating
inscriptions will all have different inscription IDs.

* Use contains_key instead of get / is_some (ordinals#3705)

* Fix typo on sat hunting page (ordinals#3668)

* Add support for mjs files (ordinals#3653)

* Use correct content type for .mjs inscriptions (ordinals#3712)

`text/javascript` is the preferred MIME type for JavaScript, so use it instead
of `application/x-javascript` when inscribing `.mjs` files.

* Remove duplicate endpoint from explorer.md (ordinals#3716)

Output `/output/<OUTPOINT>` was listed twice.

* Persist config files for ord env command (ordinals#3715)

* Add alt text to preview image (ordinals#3713)

* Fix send runes (ordinals#3484)

* Release 0.18.4 (ordinals#3720)

- Bump version: 0.18.3 → 0.18.4
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Show progress bar for etching (ordinals#3673)

* Bump rustfmt version 2018 to 2021 (ordinals#3721)

* Patch some omissions in the Chinese translation (ordinals#3694)

* Update runes.md docs (ordinals#3681)

* Update sat-hunting.md (ordinals#3724)

* Allow higher rpcworkqueue limit conf (ordinals#3615)

* Allow specifying different output formats (ordinals#3424)

* Release 0.18.5 (ordinals#3741)

* Update index.rs to fix ord list command (ordinals#3762)

* Add `ord wallet runics` command (ordinals#3734)

* Clarify teleburning.md (ordinals#3744)

* Allow postage equal to dust limit in mint.rs (ordinals#3756)

* Update runes spec (ordinals#3745)

* Fix fuzz testers  (ordinals#3740)

* Add command to list pending etchings (ordinals#3732)

* Enable resuming a specific rune etching (ordinals#3679)

* Add decode api (ordinals#3733)

* Make settings public (ordinals#3768)

* Index addresses (ordinals#3757)

* Add parents recursive endpoint (ordinals#3749)

* Remove duplicate example (ordinals#3776)

* Add /inscription/:query/:child_number route (ordinals#3777)

* Add mint progress field to rune.html (ordinals#3748)

* Add feerate percentiles to blockinfo endpoint (ordinals#3753)

* Update `ord list` to include inscriptions and runes information (ordinals#3766)

* Add delegate value to recursive inscription endpoint (ordinals#3751)

* Fix old conflicts

* Resolve issues with Ordzaar modifications

* Remove duplicate /inscriptions endpoint

* Add recursive endpoint with more details about children (ordinals#3771)

* Fix panic in ord env shutdown (ordinals#3787)

* Make recursive endpoints proxiable (ordinals#3797)

* Make settings public (ordinals#3800)

* Link address on inscription.html (ordinals#3801)

* Link address on output & tx (ordinals#3799)

* Add --http-port to settings yaml (ordinals#3796)

* Add sat balance to address page (ordinals#3810)

* Make Index public (ordinals#3807)

* Add -dev suffix to version (ordinals#3812)

* Add all transaction hex to block json response (ordinals#3805)

* Add public `shut_down()` function (ordinals#3811)

* Add typed errors with `snafu` (ordinals#3832)

* Add debugging tips README (ordinals#3823)

* Add sat name to inscription page (ordinals#3826)

* Add sat ranges to output (ordinals#3817)

* Add --all flag on `ord wallet sats` (ordinals#3824)

* Display aggregated rune balances in address page (ordinals#3831)

* Update Spanish Translation (ordinals#3835)

* Add charm to burned inscriptions (ordinals#3836)

* Add ability to cancel shutdown (ordinals#3820)

* Add inscriptions to address page (ordinals#3843)

* Release 0.19.0 (ordinals#3844)

- Bump version: 0.18.5 → 0.19.0
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Update index method name

---------

Co-authored-by: twosatsmaxi <[email protected]>
Co-authored-by: bitspill <[email protected]>
Co-authored-by: Ordinarius <[email protected]>
Co-authored-by: Dr.JingLee <[email protected]>
Co-authored-by: gmart7t2 <[email protected]>
Co-authored-by: RandolphJiffy <[email protected]>
Co-authored-by: nine <[email protected]>
Co-authored-by: Petrius Lima <[email protected]>
Co-authored-by: 0xLugon <[email protected]>
Co-authored-by: raph <[email protected]>
Co-authored-by: Casey Rodarmor <[email protected]>
Co-authored-by: tgscan-dev <[email protected]>
Co-authored-by: ynohtna92 <[email protected]>
Co-authored-by: nix.eth <[email protected]>
Co-authored-by: rongyi <[email protected]>
Co-authored-by: Felipe Lincoln <[email protected]>
Co-authored-by: blackj <[email protected]>
Co-authored-by: StevenMia <[email protected]>
Co-authored-by: Javier Villanueva <[email protected]>
Co-authored-by: oxSaturn <[email protected]>
Co-authored-by: zmeyer44 <[email protected]>
Co-authored-by: Taha Abbasi <[email protected]>
Co-authored-by: losingle <[email protected]>
Co-authored-by: Vannix <[email protected]>
Co-authored-by: knowmost <[email protected]>
Co-authored-by: Eloc <[email protected]>
Co-authored-by: bingryan <[email protected]>
Co-authored-by: AM shadow <[email protected]>
Co-authored-by: Han Tuzun <[email protected]>
Co-authored-by: Jeremy Rubin <[email protected]>
Co-authored-by: Luis Aguilar <[email protected]>
Co-authored-by: Jeason <[email protected]>
Co-authored-by: phorkish <[email protected]>
Co-authored-by: Daniel <[email protected]>
Co-authored-by: benbuschmann <[email protected]>
Co-authored-by: thewrlck <[email protected]>
Co-authored-by: Nick <[email protected]>
Co-authored-by: Young <[email protected]>
Co-authored-by: Zerone495 <[email protected]>
Co-authored-by: onchainguy <[email protected]>
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.

Premine percentage formula should use the total supply
4 participants