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

rfc(0017): revise since RFC #226

Merged
merged 15 commits into from
Sep 14, 2022
Merged

Conversation

doitian
Copy link
Member

@doitian doitian commented Feb 26, 2021

[PREVIEW]

  • Use more precise description.
  • Add diagrams and remove codes.
  • Fix errors:
    • The timestamp is in the unit milliseconds but not seconds.
    • There are many errors where <= is expected but < is used or
      vise versa.
    • The Rust code base implementation has many gotchas and must be
      considered as the part of consensus.

Actions:

  • Rewrite RFC17
  • Update RFC28
  • Update RFC30

@doitian doitian marked this pull request as ready for review February 26, 2021 23:04
@doitian doitian requested a review from a team as a code owner February 26, 2021 23:04
@doitian doitian changed the title Revise Since RFC. RFC17: Revise Since RFC Jul 23, 2021
jjyr
jjyr previously approved these changes Dec 24, 2021
@doitian doitian added the p:must-have Priority: critical to the current delivery timebox in order for it to be a success label Apr 1, 2022
@doitian doitian changed the title RFC17: Revise Since RFC rfc(0017): revise since RFC Apr 14, 2022
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
rfcs/0017-tx-valid-since/0017-tx-valid-since.md Outdated Show resolved Hide resolved
doitian and others added 9 commits August 19, 2022 10:45
- Use more precise description.
- Add diagrams and remove codes.
- Fix errors:
* The timestamp is in the unit milliseconds but not seconds.
* There are many errors where `<=` is expected but `<` is used or
vise versa.
* The Rust code base implementation has many gotchas and must be
considered as the part of consensus.
@doitian doitian requested a review from janx August 22, 2022 02:08
@doitian doitian requested a review from jjyr August 22, 2022 02:08
@janx janx requested review from jordanmack and removed request for jjyr August 27, 2022 01:47
@jordanmack
Copy link
Contributor

I will submit an additional pull request with some modifications which will make it easier to read for native English speakers. First I have some questions.

When the metric flag is set to epoch number with fraction (01), value represents a rational number E + I / L, where

E has 3 bytes, from the lowest bit 0 to 23.
I has 2 bytes, from the bit 24 to 39.
L has 2 bytes, from the bit 40 to 55.
  1. It says here that E is the "lowest bit 0 to 23". Is this left to right, and then isn't this the highest bit?
  2. I see E, I, and L are separate values contained within the value field. What is the relevance of E + I / L?
  3. If a transaction is submitted and the since precondition is not fulfilled, is the transaction always immediately rejected?

@doitian

@doitian
Copy link
Member Author

doitian commented Aug 29, 2022

I will submit an additional pull request with some modifications which will make it easier to read for native English speakers. First I have some questions.

When the metric flag is set to epoch number with fraction (01), value represents a rational number E + I / L, where

E has 3 bytes, from the lowest bit 0 to 23.
I has 2 bytes, from the bit 24 to 39.
L has 2 bytes, from the bit 40 to 55.
  1. It says here that E is the "lowest bit 0 to 23". Is this left to right, and then isn't this the highest bit?

The whole since field is an integer, the lowest bit is the least significant bit.

  1. I see E, I, and L are separate values contained within the value field. What is the relevance of E + I / L?

The whole value part is a rational number for epoch number with fraction.

  1. If a transaction is submitted and the since precondition is not fulfilled, is the transaction always immediately rejected?

Not a requirement of the RFC, but yes, in current node implementation, the tx is rejected immediately.

@jordanmack
Copy link
Contributor

  1. I see E, I, and L are separate values contained within the value field. What is the relevance of E + I / L?

The whole value part is a rational number for epoch number with fraction.

I still do not understand what this is useful for. Right now is epoch 6083 index 698 length 1028. What is 6083 + 698 / 1028 supposed to be used for?

  1. Are all values mentioned always stored in little-endian and read using LSB 0 bit numbering?

@doitian
Copy link
Member Author

doitian commented Aug 30, 2022

  1. I see E, I, and L are separate values contained within the value field. What is the relevance of E + I / L?

The whole value part is a rational number for epoch number with fraction.

I still do not understand what this is useful for. Right now is epoch 6083 index 698 length 1028. What is 6083 + 698 / 1028 supposed to be used for?

Say you set since to absolute, epoch with fraction, 500 + 1 / 2, you mean that the transaction should commit since the second half of epoch 500.

  1. Are all values mentioned always stored in little-endian and read using LSB 0 bit numbering?

This RFC does not mention the wire encoding. In block and tx binary format encoded by Molecule, all integers are encoded using little-endian. In RPC Json format, all integers are encoded in hex.

@jordanmack
Copy link
Contributor

Say you set since to absolute, epoch with fraction, 500 + 1 / 2, you mean that the transaction should commit since the second half of epoch 500.

Is E + I / L just a notation being used to describe the epoch in a human-readable format? If yes, is this notation used elsewhere?

@doitian
Copy link
Member Author

doitian commented Aug 30, 2022

Say you set since to absolute, epoch with fraction, 500 + 1 / 2, you mean that the transaction should commit since the second half of epoch 500.

Is E + I / L just a notation being used to describe the epoch in a human-readable format? If yes, is this notation used elsewhere?

Yes, it's a notation to explain how three numbers are encoded in a single integer. The epoch field in the block header adopts the same encoding method ( https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0027-block-structure/0027-block-structure.md#epoch-uint64 ).

@jordanmack
Copy link
Contributor

jordanmack commented Sep 2, 2022

Yes, it's a notation to explain how three numbers are encoded in a single integer. The epoch field in the block header adopts the same encoding method ( https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0027-block-structure/0027-block-structure.md#epoch-uint64 ).

Sorry for making you repeat yourself, but I need to clarify E + I / L to be sure I didn't miss something. I understand this as being a way that humans would communicate with each other about the epoch and fraction.

Example: 500 + 1 / 2 means that the transaction will be allowed to commit in the second half of epoch 500.

If I understand correctly, this has no mathematical usage. This means 500 + 1 / 2 = 500.5 would be of no value to a human or a machine trying to interpret this and we should never try to use the value of 500.5 since it has no meaning.

If this is all correct, then I would suggest against using E + I / L as a notation because it looks just like a mathematical formula which can be confusing.

The second question I have is about why the since value is on the transaction input. I understand this as the since value is added to an input on a transaction. The developer who creates the transaction would be the one to optionally specify the since value on each input of the transaction. All since preconditions on all inputs in the transaction must be fulfilled before the transaction can be committed. Why is it that the since value is on each individual input instead of a single since value on the transaction itself?

@janx
Copy link
Member

janx commented Sep 2, 2022

The second question I have is about why the since value is on the transaction input. I understand this as the since value is added to an input on a transaction. The developer who creates the transaction would be the one to optionally specify the since value on each input of the transaction. All since preconditions on all inputs in the transaction must be fulfilled before the transaction can be committed. Why is it that the since value is on each individual input instead of a single since value on the transaction itself?

A CKB transaction could be co-authored by multiple users, e.g. in open transactions use case. In that case a user could set conditions on his/her inputs only.

@jordanmack
Copy link
Contributor

jordanmack commented Sep 6, 2022

@doitian
Copy link
Member Author

doitian commented Sep 9, 2022

Yes, it's a notation to explain how three numbers are encoded in a single integer. The epoch field in the block header adopts the same encoding method ( https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0027-block-structure/0027-block-structure.md#epoch-uint64 ).

Sorry for making you repeat yourself, but I need to clarify E + I / L to be sure I didn't miss something. I understand this as being a way that humans would communicate with each other about the epoch and fraction.

Example: 500 + 1 / 2 means that the transaction will be allowed to commit in the second half of epoch 500.

If I understand correctly, this has no mathematical usage. This means 500 + 1 / 2 = 500.5 would be of no value to a human or a machine trying to interpret this and we should never try to use the value of 500.5 since it has no meaning and could potentially be an irrational number.

If this is all correct, then I would suggest against using E + I / L as a notation because it looks just like a mathematical formula which can be confusing.

What's your suggestion? Using rational number notation can simplify the explanation because adding and comparison works for rational numbers just like integers.

The second question I have is about why the since value is on the transaction input. I understand this as the since value is added to an input on a transaction. The developer who creates the transaction would be the one to optionally specify the since value on each input of the transaction. All since preconditions on all inputs in the transaction must be fulfilled before the transaction can be committed. Why is it that the since value is on each individual input instead of a single since value on the transaction itself?

Contracts may have conflict requirement on the since. For example, contract requires the since is at least 100 blocks, and another contract may require the since at least 1 epoch.

@jordanmack
Copy link
Contributor

jordanmack commented Sep 12, 2022

What's your suggestion? Using rational number notation can simplify the explanation because adding and comparison works for rational numbers just like integers.

The term "rational number" is indicating a single number. The field has three values that can be assembled and referred to as a single "rational number" if you want a human readable format. But these three numbers are encoded into a u64, which is also a single number.

The notation E + I / L looks like a formula to calculate something, but it is not a formula. The terminology is not incorrect, but I believe it is less confusing to explain it as three numbers then describe the epoch as a fraction that is written as E I/L. The change is subtle, but represented as fraction without the "+" makes it less likely to be mistaken for a formula.

These changes are reflected in my pull request.

@doitian doitian requested a review from janx September 13, 2022 03:00
@doitian doitian marked this pull request as draft September 13, 2022 03:00
@jordanmack jordanmack marked this pull request as ready for review September 14, 2022 00:22
@janx janx merged commit bfacb39 into nervosnetwork:master Sep 14, 2022
@doitian

This comment was marked as duplicate.

@doitian
Copy link
Member Author

doitian commented Oct 11, 2022

The email took half a month to arrive at GitHub 😂

@doitian doitian deleted the revise-since-rfc branch February 23, 2023 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p:must-have Priority: critical to the current delivery timebox in order for it to be a success
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants