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

Clean Up messages Table #1617

Open
5 tasks
ouziel-slama opened this issue Apr 9, 2024 · 9 comments
Open
5 tasks

Clean Up messages Table #1617

ouziel-slama opened this issue Apr 9, 2024 · 9 comments

Comments

@ouziel-slama
Copy link
Contributor

  • rename messages to events
  • remove command and category fields
  • add tx_hash field (to be able to easily retrieve the events of a transaction)
  • for each block the events_hash is calculated by concatenating the list of event names, of course ordered by event_index (we deliberately exclude the bindings so as not to change the hashes with each addition of a field which could prove useful to the 'future).
  • make sure that no event is forgotten, because the next addition should be considered a protocol change.
@ouziel-slama ouziel-slama self-assigned this Apr 9, 2024
@jotapea
Copy link
Contributor

jotapea commented Apr 10, 2024

Before considering this, lets just get to a stable version with the messages as they exist today. Too many changes, too fast, and only a few people making these decisions.

The message abstraction is directly connected to the CNTRPRTY transaction message. And when it is valid, it triggers other messages at the protocol. And these messages have an order.

The event abstraction has less connection to the Counterparty transaction, and considering time is non-continuous (is block by block steps), basically all "events" happen at the same time, thus their ordering is less intuitive.

Not saying this is wrong, but lets just take things slower.

@jotapea
Copy link
Contributor

jotapea commented Apr 11, 2024

Following up on CounterpartyXCP/Documentation#185 (comment):

Thinking about the ways this could be implemented:

(Note: non-message events => the newly added categories to the messages table in v10.)

  1. Keep messages table and message_index as is, now adding non-message events to this table but still having a message_index. This is the current v10.0.0.
  2. Same as above, BUT non-message events are only for debug purposes, not included in production / hash calculation.
  3. Add a new event_index primary key column to messages. So both message and non-message events have an event index, but only message events have a message index. (This could lead to renaming the table to events...)

In v9 transaction index 0 is message index 0.

In the current v10 it isn't, because non-message events are given a message_index.

I believe that as long as transaction_index 0 = message_index 0, then the implementation can be 2 or 3 (or something else?), but option 1 is missing this cohesiveness of the message concept itself. But it does make sense when treated as an "event" though.

So basically the current v10 option 1 is a step in a better direction, but I believe it still needs some adjustments so that the messages_hash is consistent to what it has represented pre-v10. An events hash could still be useful, but I wouldn't make it a replacement to the messages hash, which in my view represents the minimal data needed to prove there is consensus in the data not included in the ledger.

If the aspiration of the protocol is to be decentralized, thus maybe having multiple implementations all sharing consensus, an events_hash (which is tightly coupled to implementation details) will make it much harder to replicate the same "message hashes".

(Also seeing non-message events in the mempool table. Maybe also all part of moving into the events paradigm which does make sense, but at least these don't affect hashes. And after the new events-messages approach is finalized the mempool will just reflect whatever is decided.)

@ouziel-slama
Copy link
Contributor Author

Having messages that trigger to the creation of other messages makes no sense. The main problem is that the name of this table is wrong. It does not contain any messages. Messages are Counterparty data embedded in transactions. Transactions/messages trigger events (which modify the database). The priority is to correct the naming of these fields and this table. This is the main goal of this issue not to change the purpose and design of this table.

@jotapea
Copy link
Contributor

jotapea commented Apr 11, 2024

@ouziel-slama let me quote what you are referring to, from CounterpartyXCP/Documentation#185 (comment):

For production hash calculation I would not include anything before the CNTRPRTY transaction message, like the block and transactions. I would also not include duplicate messages like asset and balances. This is how it is done in v9, where messages can be simply described as: "an invalid transaction message doesn't trigger more messages, but a valid one does".

In the context of describing the messages table, when viewed by a non-technical-details knowledgeable user in <v10, this is literally what they SHOW. Implementation is separate.

The name of the table is not wrong for <v10. If it is, I would like to understand why you think so. But if you are changing the nature of a table to become "events", then obviously you can now say the name is wrong.

This is the main goal of this issue not to change the purpose and design of this table.

It seems like you already did, in v10 transaction_index 0 does not represent message_index 0.

Please don't say things like "makes no sense", as that is insulting. Lets be respectful and try to understand each others perspective.

@ouziel-slama
Copy link
Contributor Author

ouziel-slama commented Apr 11, 2024

If it is, I would like to understand why you think so.

This is what I explained in my previous message. In other words: by definition a message has a sender and a receiver. In the case of a transaction/message the sender is the signatory of the transaction and the counterparty node the receiver. Today what is recorded in the messages table does not correspond to the definition of a message (no sender or receiver).

@jotapea
Copy link
Contributor

jotapea commented Apr 11, 2024

You got a point with the definition of a message. What would be a better name then? Isn't event way too generic, as basically anything with a time associated to it can be considered an event.

And my main point is that "messages" as they exist in <v10 only deal with Counterparty data state, not Bitcoin data like blocks, transactions, transaction_outputs...

What can be done to continue having transaction_index 0 be <cp_data>_index 0, so that the "purpose and design of the table" is not changed?

@jotapea
Copy link
Contributor

jotapea commented Apr 11, 2024

The first definition from a dictionary:

1: a communication in writing, in speech, or by signals

Not sender or receiver, but communication. To me, this is a very appropriate name, considering the protocol IS about writing in Bitcoin.

If what the user wrote is valid, then the software writes back.

Beautiful, so make THIS my new simple messages description.

If you find a better abstraction, then it should go through the CIP process at minimum. This is literally changing consensus, which is based on the 3 hashes the software provides per block, one which is a MESSAGES hash.

Should we move the discussion to the CIP?

@adamkrellenstein
Copy link
Member

This is not a proposal for a consensus change—that's just not how consensus in Counterparty works.

@jotapea
Copy link
Contributor

jotapea commented Apr 12, 2024

How is changing the contents of a hash calculation not breaking consensus?

I'm trying to give my feedback to proposed changes (more like already done), so that the software that I decide to run on my infrastructure continues being in agreement with the "official" reference implementation.

  1. Add a new event_index primary key column to messages. So both message and non-message events have an event index, but only message events have a message index. (This could lead to renaming the table to events...)

What is wrong with this? This will make the new "events" approach be consistent with the content of messages as they exist now, where transaction_index 0 represents message_index 0.

xcp.dev exists because of the messages table. Is what I saw and made me GET IT, because it was simple, elegant, and intuitive. What I see now is no longer intuitive. My fear is that events is such a general abstraction that it will become a basket where anything goes and thus less thought will be put into keeping it minimal, simple and elegant as it exists <v10.

I think the suggestion to have separate event_index from message_index is the best of both worlds and very easy to implement.

@adamkrellenstein adamkrellenstein changed the title replace message_hash by events_hash ? Clean Up message_hash May 2, 2024
@adamkrellenstein adamkrellenstein changed the title Clean Up message_hash Clean Up messages Table May 2, 2024
@adamkrellenstein adamkrellenstein added this to the v11.01 milestone May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants