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

Decide what to do about including identical messages in the same block #226

Closed
evan-forbes opened this issue Feb 28, 2022 · 2 comments
Closed

Decide what to do about including identical messages in the same block #226

evan-forbes opened this issue Feb 28, 2022 · 2 comments

Comments

@evan-forbes
Copy link
Member

Per these initial comments #216 (comment) #216 (comment)

If two identical messages are submitted, then a block producer has the option to

  1. include two copies of the messages in the square, and both PayForMessages
  2. include one copy with only one of the PayForMessages
  3. include one copy with both of the PayForMessages, each of the PayForMessages pointing to the same data

With the mechansim introduced in #216, we are currently rejecting proposed blocks that include both identical messages, which forces either option 1 or option 3, but we still need to make a formal decision over what we want block producers to do in this scenario.
@evan-forbes evan-forbes mentioned this issue Feb 28, 2022
Merged
@evan-forbes evan-forbes changed the title Decide if what to do about including identical messages in the same block Decide what to do about including identical messages in the same block Feb 28, 2022
@adlerjohn adlerjohn moved this to TODO in Celestia Node Feb 28, 2022
evan-forbes added a commit that referenced this issue Apr 27, 2022
@evan-forbes
address #226
f3b543f
@liamsi
Copy link
Member

liamsi commented Apr 27, 2022

I think we should not forbid including the same data multiple times.

Also, on the long run, one PFD should be able to pay for multiple messages anyways. Hence, including the same message multiple times should be valid as long as the PFD pays multiple times (i.e. the paid fee covers all messages).

evan-forbes added a commit that referenced this issue May 5, 2022
@evan-forbes @liamsi @adlerjohn
Message inclusion check (#216)
b2617e4 
* convert PreprocessTxs to PrepareProposal

* add message inclusion check to ProcessProposal

* use the malleated tx decoder to decode malleated transactions

* use constant for pay for message URL

* switch from PayForMessage to PayForData

* fix integration test

* address #226

* change missed var name

Co-authored-by: Ismail Khoffi <[email protected]>

* get missed pfm vs pfd

Co-authored-by: Ismail Khoffi <[email protected]>

* pfm -> pfd

Co-authored-by: John Adler <[email protected]>

* better wording

Co-authored-by: John Adler <[email protected]>

* better wording

Co-authored-by: John Adler <[email protected]>

* message -> data

Co-authored-by: John Adler <[email protected]>

* pfm -> pfd

Co-authored-by: Ismail Khoffi <[email protected]>

* remaining pfms -> pfds

* get rid of empty space

Co-authored-by: John Adler <[email protected]>

* get rid of empty space

Co-authored-by: John Adler <[email protected]>

* log URL type mismatch

* add todo for refactoring to check for subtree roots

Co-authored-by: Ismail Khoffi <[email protected]>
Co-authored-by: John Adler <[email protected]>
@evan-forbes
Copy link
Member Author

I think we should not forbid including the same data multiple times.

this is now the case since we implemented the non-interactive defaults and #692

Repository owner moved this from TODO to Done in Celestia Node Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
No open projects
Celestia Node
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@liamsi @evan-forbes