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

HIP-1056: Block Streams #1056

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

HIP-1056: Block Streams #1056

wants to merge 29 commits into from

Conversation

rbair23
Copy link
Member

@rbair23 rbair23 commented Oct 1, 2024

Abstract

This HIP introduces a new output data format for consensus nodes, called Block Streams, that replaces the existing
event streams, record streams, state files, and signature files with one single stream.
Each block within the block stream is a self-contained entity, including every event and all transactions that were
part of the block, the state changes as a result of those transactions, and a
network signature (using a BLS threshold signature scheme we call TSS) that proves the block was signed by a
threshold of the network.

By including state changes, downstream services can seamlessly maintain and verify state alongside consensus nodes. This
enhancement fosters greater transparency and trust within the Hedera network. Any downstream service can independently
rebuild and verify the state of consensus nodes at the time of the block, verified by the TSS network signature. Using
this state, they can provide additional services such as state proofs, state snapshots, and more.

With the event information within blocks, downstream services can be reconstructed and the hashgraph algorithm replayed,
permitting anyone to verify the correct execution of the consensus algorithm and removes the need for the extra event
stream. In doing so Hedera users gain comprehensive visibility into network activities through an easily consumable
format that can be delivered with low latency.

A key design criteria is for the block stream to be easily consumed by any programming language, and with minimal
complexity or dependencies. For example, state data can be utilized for basic queries without having to reconstruct a
merkle tree.

Block streams are an upgrade to the existing RecordStream V6. Block streams restructure and aggregate the multiple
record types in record streams, including EventStream, RecordStream, and Hedera state data to produce a single
unified stream of items.

The key enhancements offered by block streams include:

  • Unified Data Stream: Block stream consolidates event streams, record streams, sidecars, and signature files into a
    single cohesive data stream.
  • State Change Data: Each block will include state change data for the given round of consensus.
  • Verifiable Proof: Each block will be independently verifiable, containing a full proof of transactions, network
    consensus and state changes.
  • Comprehensive Protobuf specification By defining blocks in protobuf, the inputs, outputs, and state changes of
    consensus nodes are clearly specified, paving the way for future implementations and greater client diversity.

With the adoption of block streams, data output by Hedera consensus nodes will be consolidated into a single, verifiable
chronicle of the network, thereby strengthening the integrity and transparency of the Hedera ecosystem.

@rbair23 rbair23 requested a review from mgarbs as a code owner October 1, 2024 20:21
Copy link

netlify bot commented Oct 1, 2024

Deploy Preview for hedera-hips ready!

Name Link
🔨 Latest commit 231a137
🔍 Latest deploy log https://app.netlify.com/sites/hedera-hips/deploys/674fb3e381b3320008071035
😎 Deploy Preview https://deploy-preview-1056--hedera-hips.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rbair23 rbair23 changed the title Block Streams HIP-1056: Block Streams Oct 1, 2024
@rbair23 rbair23 marked this pull request as draft October 1, 2024 20:25
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
retain on disk, without breaking the cryptographic integrity of the block stream. Different block node operators in
different legal jurisdictions can make different decisions about what data to retain. Or, block node operators may
subset the stream to retain minimal data and minimize storage costs.
- **Errata Handling**: Errata refers to software bugs where a node executed the transaction correctly but did not record
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative option would be the given block node to be temporarily excluded from the network of block nodes and it's block stream invalidated. Then, upon fixing the bug, the block node operator could again join the network, sync the state and continue it's normal work.

Copy link
Member

Choose a reason for hiding this comment

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

Errata are needed when a bug on the consensus node produces incorrect output. This section refers to the necessity to change the recorded block chain. I believe this has been necessary a small number of times since genesis on mainnet.

Block Nodes are not a network; each node is completely independent. Those details will be specified in a different HIP.

Copy link
Contributor

Choose a reason for hiding this comment

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

@IvanKavaldzhiev will call out the non network block node in #1081
Feel free to make comments there also

HIP/hip-1056.md Outdated
```

```protobuf
message CryptoTransferOutput {
Copy link
Contributor

Choose a reason for hiding this comment

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

The crypto create might result in auto account creation. Shouldn't we also add a record for it for those cases?

Copy link
Member

Choose a reason for hiding this comment

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

The account creation will be present in StateChanges, no additional output is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to call out parent child considerations more explicitly. Might be done but noting here as a reminder to come back in case there's room for improvement

HIP/hip-1056.md Outdated Show resolved Hide resolved
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
@Nana-EC Nana-EC self-assigned this Oct 15, 2024
HIP/hip-1056.md Outdated Show resolved Hide resolved
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
Copy link
Member

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

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

Just a few typos and wording suggestions.

HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
Copy link
Member

@poulok poulok left a comment

Choose a reason for hiding this comment

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

Got to Design for Verifiability. Will finish the rest when I can.

HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

If it is important for the diagram to match the verbage, it should include the other stream files that are being consolidated: sidecar files and signature files

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated

HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
assets/hip-blocks/block-stream-merkle.svg Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated
Each block within the block stream is a self-contained entity, including every event and all transactions that were
part of the block, the state changes as a result of those transactions, and a
_network signature_ (using a BLS threshold signature scheme we call TSS) that proves the block was signed by a
threshold of the network.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
threshold of the network.
subset of nodes whose stake accounts for more than 1/3 of the network's consensus weight.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adopted

HIP/hip-1056.md Outdated
Comment on lines 194 to 197
The block stream has a separate `block stream merkle tree` which is a merkle tree of all the items in the block stream
over the entire history of the stream. Of course, older blocks can be represented by their hash rather than the actual
full contents, so it is possible to have a very large block stream merkle tree *in concept* that is still very efficient
to use.
Copy link
Contributor

@tinker-michaelj tinker-michaelj Oct 24, 2024

Choose a reason for hiding this comment

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

Suggested change
The block stream has a separate `block stream merkle tree` which is a merkle tree of all the items in the block stream
over the entire history of the stream. Of course, older blocks can be represented by their hash rather than the actual
full contents, so it is possible to have a very large block stream merkle tree *in concept* that is still very efficient
to use.
At each block boundary, the state merkle tree is a depth two *subtree* of a `block merkle tree`, which is a binary merkle tree whose four subtrees at depth two also include the block's input items, the block's output items, and the merkle tree of the previous block. (Of course hash computation for the block `N` merkle tree reuses the hash already computed for the block `N-1` merkle tree.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Adopted

HIP/hip-1056.md Outdated
Block streams are built out of Items. Those items can be delivered in a continuous stream containing one or more blocks.
Or they can be delivered as a single block using the `Block` message type container. If a single block is delivered as
a file it expected to be either a raw protobuf `Block` message with file name of `<BLOCK_NUMBER>.blk` e.g.
`0000000000000000001.blk` . With total of 19 digits to allow for 2^63 blocks. Or a compressed file e.g.
Copy link
Contributor

@derektriley derektriley Oct 24, 2024

Choose a reason for hiding this comment

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

Currently Block file names are 36-digit strings padded with leading zeroes. FileBlockItemWriter

Copy link
Contributor

Choose a reason for hiding this comment

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

@jsync-swirlds does this match with latest considerations or are these separate node (CN & BN) considerations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed this section as it was confused for API and is more an internal block node consideration

Copy link
Member

Choose a reason for hiding this comment

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

Just for completion. I have no idea why the consensus node chose 36-digits, but we chose to restrict file names in the block node to what a Java long can represent for efficiency. It's easy enough to trim or pad the extra 17 0s, if needed.

@derektriley
Copy link
Contributor

Should this HIP at all mention/reference the behaviors around consensus node to block node communication? Will this information be covered in the block node HIP?

HIP/hip-1056.md Outdated
Comment on lines 203 to 204
Record files are streams in a blockchain, with the hash of previous file included in the next record file, ensuring
immutability. Additionally, each consensus node on the Hedera mainnet signs a hash of the record files as they are
Copy link
Contributor

@tinker-michaelj tinker-michaelj Oct 24, 2024

Choose a reason for hiding this comment

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

Suggested change
Record files are streams in a blockchain, with the hash of previous file included in the next record file, ensuring
immutability. Additionally, each consensus node on the Hedera mainnet signs a hash of the record files as they are
Record file contents are summarized in a chain of hashes, with each file's contents containing the hash of the previous file; so that the hash of the current file depends on the contents of the entire record stream. Additionally, each consensus node on the Hedera mainnet signs the hash of each record file as it is

Copy link
Member

Choose a reason for hiding this comment

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

...containing the hash of the previous file...

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated

Copy link
Member

@poulok poulok left a comment

Choose a reason for hiding this comment

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

Got further, but still not done.

can authenticate transactional data without the cost and complexity of downloading multiple signature files for
verification.
3. As a verifier for a network, I want a single, self-contained stream of data with aggregated signature signed by a
threshold of stake weights, so that I can cost-effectively confirm that the stream represents the consensus output of
Copy link
Member

Choose a reason for hiding this comment

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

We should really decide what terms we are going to use. Some places use stake weights and others simply use weight. Either is fine as long as it is used consistently. Inconsistent use will contribute to confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated to stake weight

Copy link
Member

Choose a reason for hiding this comment

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

i forgot that I said "either is fine" in this comment. I posted another comment above saying that "stake weight" is confusing. I've always thought this, but know it's a commonly used term so I must have been feeling particularly lenient the day i made this original comment. Happy to discuss offline. I don't want to cause uncecessary churn.

HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
…es, thanks derektriley

Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
…ge SLotKey and SlotValue

Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated
the block's output items, and the merkle tree of the previous block.
Of course Hash computation for the block `N` merkle tree reuses the hash computed for the block `N-1` merkle tree.

> Merkle Item Order vs Block Item Order: The order in which items appear in the block merkle tree are not the same
Copy link
Member

Choose a reason for hiding this comment

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

I'm quite confused by this paragraph. There seem to be two contradictory statements:

The order in which items appear in the block merkle tree are not the same order in which are streamed

vs

The left-to-right order of merkle leaves must be the order in which those items are encountered in the stream.

Copy link
Member

Choose a reason for hiding this comment

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

I believe the intent here is to note that items are split into multiple subtrees, and strict in-order traversal of the combined tree will not match the stream order.

HIP/hip-1056.md Outdated

> State Merkle Hash: A block contains the state hash that represents the state at the beginning of the block.
> The hash of the state at the end of the block is found in the next block. This is because it takes time to gather state
> changes of the block and hash them, waiting for this process would result in the block delivery time being held up.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> changes of the block and hash them, waiting for this process would result in the block delivery time being held up.
> changes of the block and hash them, and waiting for this process would result in the block delivery time being held up.

produced, enabling mirror nodes and other downstream services to verify that the record file was produced and attested
by a majority weight of the network nodes. A major drawback of the current design is that multiple signature files and
the address book with network weight must be collected and verified before a record file can be verified. This design
by a majority stake weight of the network nodes. A major drawback of the current design is that multiple signature files and
Copy link
Member

@poulok poulok Dec 3, 2024

Choose a reason for hiding this comment

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

I know I've said this before, but using the term "stake weight" is very strange. There stake, and there is weight. Stake is measured in hbars. Weight is unit-less and determined from total network stake. Combining the two terms into a single term muddles the two ideas. If we mean weight, just say "weight". But maybe there has been an agreement to use "stake weight" that I'm not aware of?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, there was a correction to utilize stake weight.
Let me confirm, @jsync-swirlds thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I think using stake weight everywhere is fine; we should make sure it's defined however.

Copy link
Member

Choose a reason for hiding this comment

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

Does that mean we will have 3 terms, stake, weight, and stake weight where the last two are equivalent? These seems exceedingly confusing.

Copy link
Member

Choose a reason for hiding this comment

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

I think only stake and stake weight are needed. I'm not sure why we'd need weight by itself.

Comment on lines +241 to +257
1. Block Proof - This proof proves the blockchain and illustrates that given a starting block and its proof, a given
set of transaction inputs and outputs as well as resulting state changes and a final merkle root hash was agreed upon
by the majority stake weight of the network.
In detail the Block Proof is a message in the Block Stream that contains a Ledger ID signature on the root hash of the Block Merkle
Tree, as well as all of the information necessary to, with the Block Items for that Block, reconstruct the Block Merkle
Tree, calculate the root hash, and verify the signature with the network Ledger ID.
2. Block Item Proof - This proof proves that a given object was present in a Block. This is just as powerful as proving
a value was in state. In detail, the Block Item Proof is a Merkle Proof created for a single BlockItem within a Block.
This proves that the network consensus agreed to the content of that specific item (input, output, or state change) in
that specific block. This can prove, among other values, the content of an HCS message, the state of an Account changed
in that block, or the content of a transaction submitted in that block. We can construct a BlockItem Proof from a Block
Proof and contents of the Block.
3. State Proof - This proof proves a given state value at a given block number was agreed upon by a majority stake
weight of the network. In detail, the state proof is a Merkle Proof created for a value in network consensus state at a
particular point in consensus time. This proves that the network consensus state contained a particular value at a
specific consensus timestamp. This might prove, among other values, the balance of an Account, the state of a Smart
Contract, or the properties of an HTS non-fungible token.
Copy link
Member

Choose a reason for hiding this comment

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

Fantastic descriptions! Thank you!

world time depends on internet latency and event creation rate. In Hedera mainnet today it is around 1 round per
second. In the future this could be shorter to lower latency, but would never make sense to be less than ping time.
- An `Event` is a sequence of zero or more `EventTransactions` submitted by a single node.
- An `Event` contains a sequence of zero or more `EventTransactions` submitted by a single node.
- Each `EventTransaction` is either a `SignedTransaction` or a system transaction.
- A `SignedTransaction` is transmitted as a byte array, contains the signed transaction bytes of a user-submitted
Copy link
Member

Choose a reason for hiding this comment

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

This has changes since this HIP was written. We are in the process of merging the only system transaction we have into the hedera-defined transaction structure. I believe all the transactions will be SignedTransaction and the say you differentiate a system vs user submitted transaction is looking at a flag in the TransactionBody or determining it based on the TransactionBody oneof type (StateSignatureTransaction will be one of these types)

HIP/hip-1056.md Outdated Show resolved Hide resolved
HIP/hip-1056.md Outdated Show resolved Hide resolved
@@ -1393,6 +1390,13 @@ message QueuePopChange {
}
```

> Note: The block stream design ensures consistency of singleton and queue states at round boundaries so as to reduce the
Copy link
Member

Choose a reason for hiding this comment

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

Great addition, thank you!

Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
which would be required to use *the same* deterministic iteration order when serializing bytes, so that the generated
hash value would match every other implementation. The network values the freedom to implement a compliant Block Stream
consumer using any valid protocol buffer library and any computing language of choice, and therefore chooses not to use
the `map` type in protobuf messages.
Copy link
Member

Choose a reason for hiding this comment

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

Speaking of custom protobuf implementation, it may be worth mentioning that pbj actually has support for maps that guarantees ordering (see hashgraph/pbj#258).

Copy link
Member

Choose a reason for hiding this comment

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

Everything in block stream has to work with other protobuf implementations (e.g. protoc or tonic) as well; which means we cannot use maps in Block Streams.

…ansactionResult

Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
* with this transaction</li>
* </ul>
*/
repeated proto.TokenTransferList token_transfer_lists = 3;
Copy link
Member

Choose a reason for hiding this comment

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

We need to check this. I recall something about token transfers being possible in a fairly wide range of transactions (which is why it ended up in the transaction result).

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the newest note was TransactionResult should only contain items that are applicable to all transactions, not some (even if the majority). You also noted that filtering logic wouldn't work well if this isn't adhered too.
Is there flexibility on this?

Copy link
Member

Choose a reason for hiding this comment

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

I was more noting that we may need to add the token transfer lists to a few more places.

Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
Comment on lines +6 to +8
working-group: Jasper Potts <@jasperpotts>, Richard Bair <@rbair23>, Nana Essilfie-Conduah <@Nana-EC>,
Mark Blackman <mark@swirldslabs.com>, Leemon Baird, Joseph Sinclair <@jsync-swirlds>, Nick Poorman <@nickpoorman>,
Kelly Greco <@poulok>, Steven Sheehy <@steven-sheehy>, Michael Tinker <@tinker-michaelj>
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it truly causes an issue for the hips site, but at least github rendering complains of the following:

Error in user YAML: (<unknown>): could not find expected ':' while scanning a simple key at line 4 column 1

Should not wrap the author and working-group lines just to be safe

* <p>
* This SHALL be a keccak256 hash of the ethereumData.
*/
bytes ethereum_hash = 1;
Copy link
Member

Choose a reason for hiding this comment

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

If we're not passing transaction_hash in result then should we not pass this ethereum_hash? Or vice versa if we're passing ethereum_hash shouldn't we pass transaction_hash? It should be consistent: Only upstream calculates or both upstream/downstream calculate.

Copy link
Member

Choose a reason for hiding this comment

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

If it's relatively simple for downstream to correctly calculate this (not sure, do we have the same ethereumData on both sides? The way we handle EVM is a bit opaque to me), then we definitely should leave out that easily calculated data.

* <p>
* This value SHALL be deterministic with the cause of the state change.
*/
proto.Timestamp consensus_timestamp = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Is this timestamp necessary? Won't it be the same as the preceding TransactionResult?

/**
* A key for a change affecting a map keyed by an Account identifier.
*/
proto.AccountID account_id = 1;
Copy link
Member

@steven-sheehy steven-sheehy Dec 9, 2024

Choose a reason for hiding this comment

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

The changes to the keys and value names were not also made to the separate protos in this PR. Will we be doing a mass sync from the md to the proto before merge? Or how else are we ensuring things are not getting out of sync?

Copy link
Member

Choose a reason for hiding this comment

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

We need to resync things, definitely. There are some substantial changes being worked on that, I suspect, will require a good bit of changes across all four locations. I think we'll need to resync at that point.

* A collection of RSA signatures from consensus nodes.<br/>
* These signatures validate the hash of the record_file_contents field.
*/
repeated bytes record_file_hash_signatures = 4;
Copy link

@xin-hedera xin-hedera Dec 10, 2024

Choose a reason for hiding this comment

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

shouldn't we also include information that which sigature file (as bytes) comes from which node?

Copy link
Member

Choose a reason for hiding this comment

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

Good point, we might need to define the format of these a bit better.
@jasperpotts is working on the tool to build these; any thoughts on this, Jasper?

Comment on lines +683 to +691
/**
* All Token transfers as a result of this transaction.
*/
repeated proto.TokenTransferList token_transfer_lists = 7;

/**
* All token associations implicitly created while handling this transaction.
*/
repeated proto.TokenAssociation automatic_token_associations = 8;

Choose a reason for hiding this comment

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

are we supposed to remove both token_transfer_lists and automatic_token_associations from TransactionResult? I see they have been added to individual TransactionOutput proto messages.

/**
* Output from a token airdrop transaction.
*/
TokenAirdropOutput token_airdrop = 7;

Choose a reason for hiding this comment

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

no TokenClaimAirdropOutput? Image such an output proto message should have assessed_custom_fees and token_transfer_lists.

Copy link
Member

@jsync-swirlds jsync-swirlds Dec 10, 2024

Choose a reason for hiding this comment

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

Airdrop is special, claim does not charge fees, because the entity that sent the airdrop is required to pre-pay custom fees, association fees, and one increment of rent on the association.
I believe the state changes covers all transfers as a result.

represented as a binary merkle tree with 4 sub-trees at depth 2 which create the block proof and tree when hashed to
obtain a merkle root.

![Root Hashes](../assets/hip-1056/root_hashes.svg)

Choose a reason for hiding this comment

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

Suggested change
![Root Hashes](../assets/hip-1056/root_hashes.svg)
![Root Hashes](../assets/hip-1056/root-hashes.svg)

The SVG file has two Root of Input Merkle Tree, and so missing Root of Output Merkle Tree

/**
* Contract bytecode after deployment
*/
bytes runtime_bytecode = 3;
Copy link
Member

@steven-sheehy steven-sheehy Dec 11, 2024

Choose a reason for hiding this comment

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

Won't this already be available in the state changes? Same for eth.

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.