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

Add sequence number #2460

Merged
merged 11 commits into from
Sep 25, 2023
Merged

Add sequence number #2460

merged 11 commits into from
Sep 25, 2023

Conversation

raphjaph
Copy link
Collaborator

@raphjaph raphjaph commented Sep 20, 2023

Adds an unstable sequence number to make some logic cleaner

  • maybe remove sequence number from front-end

@raphjaph raphjaph marked this pull request as draft September 20, 2023 20:30
@@ -23,6 +23,8 @@ <h1>Inscription {{ self.number }} (unstable)</h1>
<dt>parent</dt>
<dd><a class=monospace href=/inscription/{{ parent }}>{{ parent }}</a></dd>
%% }
<dt>sequence number</dt>
<dd>{{ self.sequence_number }}</dd>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that people don't want to add a sequence number below a certain activation number to keep the original numbers prominent socially. What if we didn't show the sequence number below a threshold and instead just showed that if you run the API? We would still index both but we would only show inscription number for earlier inscriptions.

%% if self.inscription_number >= 50,000,000 {
<dt>sequence number</dt>
  <dd>{{ self.sequence_number }}</dd>
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

Another idea maybe we think of this like an engineering tolerance.

%% if self.inscription_number >= 50,000,000 {
<dt>sequence number</dt>
  <dd>+ {{ self.sequence_number - self.inscription_number }}</dd>
  }
  elseif self.inscription_number < 50,000,000 &&  self.inscription_number > 0{
  <dt>sequence offset</dt>
  <dd>+ {{ self.sequence_number - self.inscription_number }}</dd>
  }
  elseif self.inscription_number < 0{ 
    <dt>sequence number</dt>
  <dd>+ {{ self.sequence_number }}</dd>
  }
  

Copy link
Collaborator

Choose a reason for hiding this comment

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

We specifically want to display sequence numbers so that people understand exactly how inscription numbers would change if they were made unstable. Right now people are pretty much in the dark.

Copy link
Contributor

Choose a reason for hiding this comment

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

The sentiment I found on all the twitter spaces is people don't want to change the prior inscription numbers at all behind a certain activation number. Their view is the trust in the entire protocol will be broken if we decide to change these numbers and make them unstable and we changed to a sequence number.

I think the solution that a lot of people are in agreement with on both sides is @huuep comment:
#2458 (reply in thread)
He proposes that we immutably store the prior inscription numbers using an inscription. Then at a certain inscription number activation we will then move forward with immutable sequence numbers.

I tend to agree with you. However I want to make sure the other voices are heard here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah gonna remove it from the front-end

@raphjaph raphjaph marked this pull request as ready for review September 21, 2023 20:52
src/index.rs Outdated Show resolved Hide resolved
src/index/updater.rs Outdated Show resolved Hide resolved
src/index/updater.rs Outdated Show resolved Hide resolved
src/subcommand/server.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@casey casey left a comment

Choose a reason for hiding this comment

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

LGTM except for comments

@BitTogether
Copy link

Please add this, we need to have one numbering system that includes both sets of ordinals without the positive negative gimmick thing.

@raphjaph raphjaph merged commit 528ecab into master Sep 25, 2023
6 checks passed
@raphjaph raphjaph deleted the sequence-number branch September 25, 2023 18:38
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.

None yet

4 participants