feat(consensus): implement kip-15 ( DONT MERGE before refactoring a description .... ) by reshmem · Pull Request #1 · IgraLabs/rusty-kaspa

reshmem · 2025-02-08T19:51:15Z

Accepted-ID-MR = Hash ( SP.Accepted-ID-MR, MR of Accepted-TXs ) as described in https://github.com/svarogg/kips/blob/kip-0015.md/kip-0015.md

Will be happy for a review. 2 things to pay attention to:

I am not 100% about DAA for HF activation. I mean I am not 100% sure I took the right value in both places. Please double check me.
The Hash computation, maybe it worth moving the computation code to some place under new function name ?

reshmem · 2025-02-08T19:54:58Z

consensus/src/pipeline/virtual_processor/processor.rs

        let hash_merkle_root = calc_hash_merkle_root(txs.iter(), storage_mass_activated);

-        let accepted_id_merkle_root = kaspa_merkle::calc_merkle_root(virtual_state.accepted_tx_ids.iter().copied());
+        let accepted_id_merkle_root = if self.accepted_id_merkle_root.is_active(virtual_state.daa_score) {


@svarogg - here DAA_SCORE comes from Virtual-State

reshmem · 2025-02-08T19:55:39Z

consensus/src/pipeline/virtual_processor/utxo_validation.rs

        // NOTE: when subnetworks will be enabled, the sort should consider them in order to allow grouping under a merkle subtree
-        ctx.accepted_tx_ids.sort();
+        // Preserve canonical order of accepted transactions after hard-fork
+        if !self.accepted_id_merkle_root.is_active(pov_daa_score) {


Hera POV_DAA_SCORE is equal to header.daa_score of the caller

@svarogg ...

reshmem · 2025-02-08T19:56:02Z

consensus/src/pipeline/virtual_processor/utxo_validation.rs


        // Verify header accepted_id_merkle_root
-        let expected_accepted_id_merkle_root = kaspa_merkle::calc_merkle_root(ctx.accepted_tx_ids.iter().copied());
+        let expected_accepted_id_merkle_root = if self.accepted_id_merkle_root.is_active(header.daa_score) {


@svarogg - Here as well, header.daa_score

reshmem · 2025-02-08T19:56:39Z

consensus/src/pipeline/virtual_processor/utxo_validation.rs

        // Verify header accepted_id_merkle_root
-        let expected_accepted_id_merkle_root = kaspa_merkle::calc_merkle_root(ctx.accepted_tx_ids.iter().copied());
+        let expected_accepted_id_merkle_root = if self.accepted_id_merkle_root.is_active(header.daa_score) {
+            kaspa_merkle::merkle_hash(


@svarogg - No sure if it better to have new function that computes it....

pycckuu

Here is my 2 cents. 😃

I am not sure If applicable, should we add a task to write boundary tests around the splits:

Boundary block validation to verify that the last pre-activation block enforces sorting, while the first post-activation block uses chained merkle roots
Test chain reorganizations across the activation boundary to prevent invalid blocks

pycckuu · 2025-02-09T04:21:17Z

consensus/core/src/config/params.rs

    pub runtime_sig_op_counting: ForkActivation,
+
+    /// Canonical transaction ordering
+    pub accepted_id_merkle_root: ForkActivation,


I would add documentaion referene:

/// KIP-15 activation: When enabled, uses chained merkle roots with canonical tx ordering pub accepted_id_merkle_root: ForkActivation,

@pycckuu - I am not sure about the name in general - maybe ForkChoice should be kip15_activation or something like this.

pycckuu · 2025-02-09T04:26:51Z

consensus/src/pipeline/virtual_processor/processor.rs

-        let accepted_id_merkle_root = kaspa_merkle::calc_merkle_root(virtual_state.accepted_tx_ids.iter().copied());
+        let accepted_id_merkle_root = if self.accepted_id_merkle_root.is_active(virtual_state.daa_score) {
+            kaspa_merkle::merkle_hash(
+                self.headers_store.get_header(virtual_state.ghostdag_data.selected_parent).unwrap().accepted_id_merkle_root,


I noticed a potential issue in the code where we're using unwrap() on the parent header retrieval. This could cause panics if the header is missing. I suggest replacing it with proper error handling:

let parent_header = self.headers_store.get_header(...) .map_err(|_| RuleError::MissingParentHeader(selected_parent))?;

This change adds explicit error handling for header validation and prevents node crashes.

Thank you. Good point.... I will check if this situation is possible in general. Looks like from POV of virtual selected-parent should exist. Always. But I will check with core-team, if we should handle this error explicitly ( maybe it is implicitly assumed in this context ). @svarogg ? what do think ?

@pycckuu - I asked kaspa core-team, and indeed in this context it is safe to call unwrap without error handling. S.P header is there ( specifically in this context )

pycckuu · 2025-02-09T06:04:41Z

consensus/src/pipeline/virtual_processor/utxo_validation.rs

-        ctx.accepted_tx_ids.sort();
+        // Preserve canonical order of accepted transactions after hard-fork
+        if !self.accepted_id_merkle_root.is_active(pov_daa_score) {
+            ctx.accepted_tx_ids.sort();


May we sort_unstable() instead?

Suggested change

ctx.accepted_tx_ids.sort();

ctx.accepted_tx_ids.sort_unstable();

It's faster than regular sort, though it won't preserve order of equal elements. It is not important as we shouldn't have duplicates.

It was originally sorted this way, so I should preserve past behaviour, otherwise network will split ....

…ion tests to calc storage mass correctly

* kip13 * inline tracking per mass unit * various small fixes * docs

…d not pov daa score

* implement storage mass plurality * move plurality calculation to mass module and add a useful trait for accessing it * utxo cell * consume outputs iterator only once, fix theoretic overflow * add sanity check for plurality limits, align comments * optimize input iterator consumption * extended comment rewrite * fix doc build * inline the relaxed formula conditions * test_storage_mass_pluralities

kaspanet#642) * change expected pruning point check from header validity to chain qualification * adjust `pruning points valid chain` verification to the new rule change

1. Accepted-ID-MR = Hash ( SP.Accepted-ID-MR, MR of Accepted-TXs )

reshmem requested review from nowinter, pycckuu, svarogg and vkonst February 8, 2025 19:51

reshmem commented Feb 8, 2025

View reviewed changes

pycckuu approved these changes Feb 9, 2025

View reviewed changes

pycckuu reviewed Feb 9, 2025

View reviewed changes

michaelsutton added 16 commits February 10, 2025 16:33

deactivate tn11

121d7b7

wip fork params

2c7a2ca

wip fork params (2)

aad0788

we are not forking the timestamp deviation tolerance param

d3e8d85

min difficulty len can be defined only once

f592f3b

forked param

58a5920

use only a single (crescendo) activation flag. fix tx related activat…

cf29cab

…ion tests to calc storage mass correctly

rename _len -> _size for consistency

070eb50

wip bps forking

da490e3

wip bps forking: filter non activated blocks from DAA window

b0d316f

bps forking logic

82881f2

fix gated feature tests

72039c3

media time window should exclude non activated blocks as well

9524dc4

unused

39f14f1

ghostdag k forking logic

273d9de

coinbase reward forking logic

e817f3c

reshmem force-pushed the canonical_tx_order_kip branch from 1d90b63 to 58290f7 Compare February 12, 2025 17:00

michaelsutton added 3 commits February 16, 2025 12:52

parents and mergeset size limits forking logic

d1eb348

merge depth and finality depth forking logic

8aea539

pruning point forking logic

bdd2c78

michaelsutton and others added 25 commits February 17, 2025 21:01

fix doc test

49fdcdc

various todos and leftovers

e3d386a

mempool-related forking logic, more todos

8257064

KIP-13 (part of crescendo) (kaspanet#639)

02f71c5

* kip13 * inline tracking per mass unit * various small fixes * docs

ghostdag: comments; extract var and reuse; handle origin edge case

ace9b8f

permanent -> persistent

3427aa1

use bool -> option -> iter and remove code duplication

487b794

coinbase misc

7efd014

header misc

e96f232

comments

d614b5d

log the hardfork (initial)

1d8e3b9

fix block depth origin case

6198d8a

Merge branch 'master' into crescendo

60a66b3

initial ascii art + human readable difficulty print

d796823

countdown log

500245d

utxo-related forking activation should be based on block daa score an…

8e55bb4

…d not pov daa score

todo

4f7b7fb

handle edge cases related to trusted blocks being checked for activation

37bf2e8

Change expected pruning point to chain qualification check (Crescendo) (

8b5232a

kaspanet#642) * change expected pruning point check from header validity to chain qualification * adjust `pruning points valid chain` verification to the new rule change

tx limits forking logic

cc65bff

coinbase maturity forking logic

c5b43ca

feat(consensus): implement kip-15

553796d

1. Accepted-ID-MR = Hash ( SP.Accepted-ID-MR, MR of Accepted-TXs )

fix(consensus): after rebase ...

39879b3

fix: after review

7909a97

reshmem force-pushed the canonical_tx_order_kip branch from 58290f7 to 7909a97 Compare February 26, 2025 11:54

	ctx.accepted_tx_ids.sort();
	ctx.accepted_tx_ids.sort_unstable();

Conversation

reshmem commented Feb 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reshmem Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reshmem Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pycckuu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pycckuu Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reshmem Feb 8, 2025 •

edited

Loading

reshmem Feb 8, 2025 •

edited

Loading

pycckuu left a comment •

edited

Loading

pycckuu Feb 9, 2025 •

edited

Loading