Skip to content

refactor: abstract help fn for justifications#213

Merged
g11tech merged 10 commits intoblockblaz:mainfrom
GrapeBaBa:justification
Sep 21, 2025
Merged

refactor: abstract help fn for justifications#213
g11tech merged 10 commits intoblockblaz:mainfrom
GrapeBaBa:justification

Conversation

@GrapeBaBa
Copy link
Copy Markdown
Member

This pull request refactors how the justifications map is handled in the process_attestations function of transition.zig. The main improvements are the extraction of logic into helper functions for loading and flattening the justifications map, and making memory management more explicit and robust. These changes improve code readability, maintainability, and safety.

Fix #182

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the justifications map handling in the process_attestations function by extracting the logic into two helper functions: loadJustificationsMap and flattenJustificationsMap. The changes improve code organization, memory management, and error handling while maintaining the same functionality.

Key changes:

  • Extracted justifications map initialization into loadJustificationsMap helper function
  • Extracted justifications map flattening into flattenJustificationsMap helper function
  • Changed from managed to unmanaged hash map for more explicit memory control

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +209 to +214
try justifications.put(allocator, vote.target.root, targetjustifications);
break :targetjustifications targetjustifications;
};

target_justifications[validator_id] = 1;
try justifications.put(vote.target.root, target_justifications);
try justifications.put(allocator, vote.target.root, target_justifications);
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The put method for std.AutoHashMapUnmanaged requires only two parameters (key and value), but three parameters are being passed here. The allocator should not be passed to the put method directly.

Copilot uses AI. Check for mistakes.
Comment on lines +209 to +214
try justifications.put(allocator, vote.target.root, targetjustifications);
break :targetjustifications targetjustifications;
};

target_justifications[validator_id] = 1;
try justifications.put(vote.target.root, target_justifications);
try justifications.put(allocator, vote.target.root, target_justifications);
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The put method for std.AutoHashMapUnmanaged requires only two parameters (key and value), but three parameters are being passed here. The allocator should not be passed to the put method directly.

Copilot uses AI. Check for mistakes.

std.debug.assert(end_idx <= state.justifications_validators.len);

justifications.put(allocator, blockRoot, state.justifications_validators[start_idx..end_idx]) catch |e| {
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The put method for std.AutoHashMapUnmanaged requires only two parameters (key and value), but three parameters are being passed here. The allocator should not be passed to the put method directly.

Suggested change
justifications.put(allocator, blockRoot, state.justifications_validators[start_idx..end_idx]) catch |e| {
justifications.put(blockRoot, state.justifications_validators[start_idx..end_idx]) catch |e| {

Copilot uses AI. Check for mistakes.
GrapeBaBa and others added 4 commits September 19, 2025 21:43
…blaz#198)

* fix: make start node test friendly and fix several memory leak

Signed-off-by: Chen Kai <281165273grape@gmail.com>

* Update pkgs/types/src/lib.zig

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix: fix more memory leak when error

Signed-off-by: Chen Kai <281165273grape@gmail.com>

* fix: fix one more error meory leak

Signed-off-by: Chen Kai <281165273grape@gmail.com>

* Update pkgs/cli/src/node.zig

* Update pkgs/cli/src/main.zig

* fix: flattening order (blockblaz#208)

* update sig size to 4000 bytes as per spec (blockblaz#210)

* fix: abstract away network args into a function

Signed-off-by: Chen Kai <281165273grape@gmail.com>

---------

Signed-off-by: Chen Kai <281165273grape@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: g11tech <develop@g11tech.io>
Co-authored-by: Anshal Shukla <53994948+anshalshukla@users.noreply.github.com>
Co-authored-by: g11tech <gajinder@zeam.in>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
@g11tech
Copy link
Copy Markdown
Member

g11tech commented Sep 19, 2025

to be merge and rebased on top of this PR:

so this PR waits till that time

@g11tech g11tech added the stf label Sep 21, 2025
@g11tech g11tech merged commit 1f7ac11 into blockblaz:main Sep 21, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

abtract out creation and re-flattening of justifications map into helper fns

4 participants