Aurora block hashchain #705

Casuso · 2023-02-24T21:55:55Z

Description

Aurora Block Hashchain

This implementation correlates to AIP 8.

Hashchain is turned off by default. We need to call start_hashchain on the contract to turned it on. More on this in the additional information below.

We are using a Merkle tree to compute the transactions hash of a block. The tree is actually never constructed; we dynamically maintain only the growing branch, storing hashes of the fully completed subtrees.

The hierarchy of usage is:
-contract (engine/src/lib.rs): Uses BlockchainHashchain to send txs and get the block hashchain.
-standalone: Uses BlockchainHashchain to send txs.
-BlockchainHashchain: Keeps track of block hashchain and other info, and uses BlockHashchainComputer to add txs and get hashes.
-BlockHashchainComputer: Keeps track of the StreamCompactMerkleTree and computes hashes.
-StreamCompactMerkleTree: Maintains the growing subtrees and compact subtrees applying Merkle tree rules.

Performance / NEAR gas cost considerations

To measure the impact on gas consumption of the block hashchain computation, we ran four tests against a base branch and the hashchain branch. After getting the raw gas consumption results from both, we found the delta (difference) between them and used some statistics to show the impact.

Tests 1 and 2 are focused on executing transactions on the same block, which is the most common case as only one transaction triggers the change in block height. For these the average increase observed (delta ave) is less than 0.46 Tgas, and the maximums are less than 0.64 Tgas.

Tests 3 and 4 are focused on executing a transaction on a block height change, that triggers the block hashchain computation, which is the most extensive one. Both show average and max increases of less than 0.62 Tgas.

An increase of 0.64 or 0.62 Tgas is a small amount so we should be fine. For reference, the transaction gas limit is 300 Tgas.

Testing

Several tests to cover the described structures.

Existing tests are sufficient to correlated the state between contract and standalone.

How should this be reviewed

Specifically, I would like to get feedback on the contract entry points methods on engine/src/lib.rs.

I want to make sure that we are reading correctly the input and output in all the methods that include the hashchain update. This is critical since a small difference on the input or output would imply a different hashchain value for all the blockchain.

Also, I want to make sure that we are covering all and only all the methods in the contract that should include the hashchain. The list of all the contract methods with their inclusion or exclusion of hashchain will be added as a comment on this PR so you can review it.

Additional information

The discussed mechanism to start the hashchain was:

We should merge this PR with the hashchain off.
A hashchain seed value should be separately precalculated (and continually updated) starting from Aurora Genesis block.
Aurora DAO should pause the contract. There is a separate PR for that. The hashchain seed value should include that last tx.
Aurora Labs should call start_hashchain passing the hashchain seed with its corresponding block height. This method adds its own data to the hashchain since it changes the state.
It also resumes the contract, we don't need another call to resume.
We can remove the methods pause, resume or start hashchain if decided.

TODO:

Add an account requirement (from Aurora Labs) in the start_hashchain method.
Merge the pause contract PR and add hashchain update to pause and resume methods since they change the state.

… type.

engine/src/lib.rs

… after empty Aurora blocks.

This reverts commit 860361e.

…s called after empty Aurora blocks." This reverts commit 512c8bf.

engine/src/lib.rs

engine/src/hashchain.rs

engine-standalone-storage/src/sync/mod.rs

engine/Cargo.toml

engine/src/bloom.rs

engine/src/hashchain.rs

joshuajbouw · 2023-07-18T21:07:15Z

Adding do not merge until it is ready for release.

…putation." This reverts commit 2d13411.

birchmd

Requesting changes because there are a few outputs that are not correctly captured by the hashchain. Please also resolve conflicts to be up to date with the latest develop branch.

engine-standalone-storage/src/sync/mod.rs

engine/src/hashchain.rs

engine/src/lib.rs

birchmd · 2023-07-25T19:15:17Z

engine/src/lib.rs

@@ -547,33 +688,43 @@ mod contract {
                &mut Runtime,
            );
        }
+
+        update_hashchain(&mut io, function_name!(), &input, &[], &Bloom::default());


We need to be careful about the outputs. The output is not empty in the case of this function. You can see the io object is used inside receive_erc20_tokens. I think I mentioned this before, but I still wonder if it makes sense to try and connect the hashchain with the IO implementation of NearRuntime so that we never miss outputs.

I think the issue we ran into before is how to pass in the additional information (input, function name, bloom) without breaking the IO API. I'm wondering if there is a way to do a sort of builder pattern so that each call to the NearRuntime object updates the piece of the hashchain it knows about, but maybe that will be awkward and not work out well either.

In any case, if we are manually passing outputs then we will need to be very careful that all outputs are properly captured.

As per our conversation, capturing the output here and in others would require an investigation of how they look on the standalone side (get_output_and_log_bloom method).

The idea of a builder pattern should be the best for a strong long-term solution. I think it would be better if the pattern gets implemented at the IO trait level or something slightly below it, so it has a wide impact both on the contract and on the standalone.

birchmd · 2023-07-25T19:27:46Z

engine/src/lib.rs

@@ -946,15 +1161,18 @@ mod contract {
            // that they over paid for their deposit.
            unsafe { io.promise_create_batch(&promise) };
        }
+        update_hashchain(&mut io, function_name!(), &input, &[], &Bloom::default());


This function also has a non-empty output. And similarly for the other storage_* functions.

As per our conversation, capturing the output here and in others would require an investigation of how they look on the standalone side (get_output_and_log_bloom method).

Casuso · 2023-07-27T16:36:48Z

engine/src/lib.rs

+        let mut state = state::get_state(&io).sdk_unwrap();
+
+        // *** TODO requires some Aurora Labs account
+        // require_account(some_AuroraLabs_account);


Please add a require_account statement here for an account owned by Aurora Labs. For security reasons, it's better that I don't create the account.

birchmd

The hashchain implementation still needs a little more iteration as discussed in the comments above regarding how outputs are captured. However, maintaining a feature branch like this with significant code changes requires a lot of work. Given that this PR will have no effect on future Engine deployments until the hashchain feature is enabled in the state by the DAO pausing the contract and us calling start_hashchain, I propose that we merge this PR and iterate on the implementation in smaller follow-up PRs.

To reiterate, my argument is that is safe to merge this PR (all functionality is runtime-gated in a way that only the DAO can enable) and that not merging this PR creates greater maintenance overhead for finishing the work. Therefore merging it now seems better to me than continuing development on the feature branch.

Let me know what you think @joshuajbouw @aleksuss @mandreyel

#810) ## Description This is the first in a series of PRs that is meant to split up #705 . The idea is to merge the changes which are made in that PR in logical chunks until eventually the whole hashchain implementation is in. Doing the work in smaller pieces will both make it easier to review and prevent us from needing to maintain large, long-lived feature branches. This first PR pulls in the transaction transaction parsing logic from [borealis-engine-lib](https://github.com/aurora-is-near/borealis-engine-lib) into this repo (in a future PR we will remove the duplicated code from borealis-engine-lib). The logic is used here to simplify how transactions are handled in tests because all transactions can automatically be passed to both the Near runtime (processed by the Engine as Wasm) and the standalone engine. In particular we remove the large `if` statement that was starting to get unwieldy. This work is important both because it makes future tests easier to write and because it synchronizes the standalone and wasm engine instances in our tests (this latter point is a prerequisite for properly testing the hashchain). Some notes about the PR: 1. I renamed the constant `ORIGIN` to `DEFAULT_AURORA_ACCOUNT_ID` because I felt the latter is a more descriptive name for what the constant represents. 2. The standalone engine is now present in `AuroraRunner` by default to make out testing more robust (for example tests will now automatically fail if a new state-mutating method is added to the Engine's `lib.rs` without also being added to the standalone implementation). This means there are a few places were I need to explicitly remove the standalone instance where `AuroraRunner` is used as something other than an Engine instance (modexp and xcc tests). 3. Some tests use view calls to inspect the Engine state and make assertions. These calls are not present in the standalone because it only cares about transactions that mutate the state. To make view calls in `AuroraRunner` it is now required to call `.one_shot()` before the calls. This essentially tells the testing framework that you are making a view call so no state modifications will be made and therefore we can ignore the standalone.

## Description This PR continues the effort of merging #705 in multiple pieces. This PR introduces the core hashchain logic as a library crate. The reasons to factor the code in this way are: 1. Allows this PR to be safely merged because it has no impact on existing engine code 2. The same core logic will be reusable between components that will perform the hashchain computation in the future, namely: Aurora Engine smart contract, standalone engine, Borealis Refiner. In the next PR I'll pull in the changes from #705 which actually introduce the hashchain calculation into the Aurora Engine smart contract and standalone engine. ## Performance / NEAR gas cost considerations N/A ## Testing New hashchain tests --------- Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>

birchmd · 2023-09-01T17:37:16Z

Closing in favour of #831

Casuso added 21 commits February 16, 2023 10:19

Comments

096067e

hashchain.rs file

5572a74

Direct parameters for hashing

0d08ffe

Adding more comments

8e064c2

More comments

50ee8dc

Merkle tree tests

6f0f155

Clippy

92385dc

Use RawH256 instead of H256

0c184e7

Borsh

4894c0b

Using RawH256 instead of H256

ef60aa4

Block Hashchain Computer tests

6c034d6

Misspellings

a7d5a1b

Cargo fmt

f917b56

Including chain id on hashchain and using standard contrac account id…

06b59be

… type.

Blockchain Hashchain

650aa3f

Blockchain Hashchain tests

ef73aad

Simplifiying add_block_tx

e7ad969

Storage get and set

eb4efd5

function update_hashchain

c457b75

lib.rs

d97a376

Format

1f41ac1

birchmd reviewed Feb 27, 2023

View reviewed changes

engine/src/lib.rs Outdated Show resolved Hide resolved

engine/src/lib.rs Outdated Show resolved Hide resolved

engine/src/lib.rs Outdated Show resolved Hide resolved

engine/src/lib.rs Outdated Show resolved Hide resolved

Casuso added 7 commits February 27, 2023 09:47

Fixing the build

68a0356

Fixes

860361e

Solving comments. Fix for when get_previous_block_hashchain is called…

512c8bf

… after empty Aurora blocks.

Revert "Fixes"

f28eb84

This reverts commit 860361e.

Revert "Solving comments. Fix for when get_previous_block_hashchain i…

6d70f96

…s called after empty Aurora blocks." This reverts commit 512c8bf.

Adding fixes again

aaa1630

Adding hashchain logic to entry point methods of Aurora block txs.

bfd4555

Casuso commented Feb 28, 2023

View reviewed changes

Casuso added 4 commits July 13, 2023 11:32

Adding Hashchain to Pause and Resume.

5f514a5

Adding resume logic in start hashchain.

eeacbd5

Comment.

0d220e8

Adding resume check in pause test.

eef8c46

aleksuss reviewed Jul 17, 2023

View reviewed changes

Casuso added 2 commits July 17, 2023 22:00

Remove borsh direct dependency and use aurora-engine-types borsh.

735b40f

Remove contract_account_id and method_name from hashchain computation.

2d13411

joshuajbouw added the S-do-not-merge Status: Do not merge label Jul 18, 2023

Revert "Remove contract_account_id and method_name from hashchain com…

692ddb5

…putation." This reverts commit 2d13411.

birchmd requested changes Jul 25, 2023

View reviewed changes

Casuso added 5 commits July 26, 2023 09:35

Merge with develop.

47f375c

Fix build.

547c1f3

Format

e5d26b8

Adding hashchain to relayer_key methods.

8615c40

Solving some PR comments.

0a53470

Casuso commented Jul 27, 2023

View reviewed changes

Clippy and Format.

5b86749

birchmd approved these changes Jul 27, 2023

View reviewed changes

Merge branch 'develop' into aurora_block_hashchain

5480f08

birchmd mentioned this pull request Aug 2, 2023

Feat(standalone): logic for parsing TransactionKind from raw Near data #810

Merged

birchmd mentioned this pull request Aug 8, 2023

Feat: core hashchain logic #816

Merged

birchmd closed this Sep 1, 2023

aleksuss deleted the aurora_block_hashchain branch October 16, 2023 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aurora block hashchain #705

Aurora block hashchain #705

Casuso commented Feb 24, 2023 •

edited

Loading

joshuajbouw commented Jul 18, 2023

birchmd left a comment

birchmd Jul 25, 2023

Casuso Jul 27, 2023

Casuso Jul 27, 2023 •

edited

Loading

birchmd Jul 25, 2023

Casuso Jul 27, 2023

Casuso Jul 27, 2023

birchmd left a comment

birchmd commented Sep 1, 2023

Aurora block hashchain #705

Aurora block hashchain #705

Conversation

Casuso commented Feb 24, 2023 • edited Loading

Description

Performance / NEAR gas cost considerations

Testing

How should this be reviewed

Additional information

joshuajbouw commented Jul 18, 2023

birchmd left a comment

Choose a reason for hiding this comment

birchmd Jul 25, 2023

Choose a reason for hiding this comment

Casuso Jul 27, 2023

Choose a reason for hiding this comment

Casuso Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

birchmd Jul 25, 2023

Choose a reason for hiding this comment

Casuso Jul 27, 2023

Choose a reason for hiding this comment

Casuso Jul 27, 2023

Choose a reason for hiding this comment

birchmd left a comment

Choose a reason for hiding this comment

birchmd commented Sep 1, 2023

Casuso commented Feb 24, 2023 •

edited

Loading

Casuso Jul 27, 2023 •

edited

Loading