feat: API method for adding stored block by ivannov · Pull Request #2738 · hiero-ledger/hiero-block-node

ivannov · 2026-05-07T14:15:46Z

Reviewer Notes

Adds addBlockRange(LongRange, BlockRangeType) to ApplicationStateFacility and implements it in BlockNodeApp, giving plugins a standard way to report which blocks they have stored or can serve.

Two ConcurrentLongRangeSet fields (storedBlocks, availableBlocks) are maintained in the implementation class to track the two types of sets: stored and available. Every BLOCK_RANGE_PERSIST_INTERVAL (1000) blocks accumulated across all calls, both sets are written to disk in a compact binary format. The sets are also persisted unconditionally on stopApplicationStateFacility() so no ranges are lost at shutdown regardless of where the counter is. On startup, loadApplicationState reads both files back via the same binary format.

Three new config properties are added to the ApplicationStateConfig: tssDataFilePath (actually renamed from dataFilePath), storedBlocksFilePath, and availableBlocksFilePath, all with defaults under /opt/hiero/block-node/node/.

Application State Configuration section was added to configuration.md covering all five properties.

Related Issue(s)

Closes #2702

codacy-production · 2026-05-07T14:17:23Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 22 complexity · 0 duplication

Metric Results

Complexity 22

Duplication 0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

ata-nas · 2026-05-07T14:42:17Z

+    public void addBlockRange(LongRange blockRange, BlockRangeType blockRangeType) {
+        // Every range is stored; only AVAILABLE ranges are both stored and available.
+        storedBlocks.add(blockRange);
+        if (blockRangeType == BlockRangeType.AVAILABLE) {
+            availableBlocks.add(blockRange);
+        }
+        // Trigger a persist whenever the running total crosses a BLOCK_RANGE_PERSIST_INTERVAL boundary.
+        final long blocksToAdd = blockRange.size();
+        final long before = storedBlockCount.getAndAdd(blocksToAdd);
+        if (before / BLOCK_RANGE_PERSIST_INTERVAL < (before + blocksToAdd) / BLOCK_RANGE_PERSIST_INTERVAL) {
+            persistBlockRanges();
+        }


This, being called from multiple threads will conflict on write (persist to disk), I believe.

I made the method synchronized

I do not believe this is the solution. The application state facility receive all kinds of updates. All updates must be made known to plugins. We can only ever do that, reasonably, if the next update comes after everyone has been notified of the previous one. There is a scheduled task, currently, that publishes all updates. I believe that is the correct place to do any update. As far as persistence, it must also be done in a similar fashion, making sure that no conflicts are possible. Making this method synchronized is quite the performance hit.

The way I see it conceptually, others could disagree, is that updates are being queued up. Then, each update is being published to everyone in the order things were queued up, by a single thread. In this way, we ensure that no update will be left unprocessed, but also that the end result, which is a block node context, will be in a state that has all the updates and none have been overwritten.

Persisting the block ranges is a different thing than updating other plugins with the block ranges. It is there just to store the current state on a regular basis, so that the next time the block node starts, it is retrieved.

As I wrote in another comment, how plugins get the information is beyond the scope of this task

The persist call is now moved to the thread, which persist the other two values managed by the application state. The method is no longer synchronized.

The following is informational - to be used in future work.
As to how to manage many updates, I would go a step further than Atanas approach.
I would recommend gathering updates until some criteria (time, specific update, volume of updates, etc...) is met, then push a single update representing the current state of the application data.
This reduces update frequency while also allowing for immediate response when needed.
We can workshop further in future work (probably when we extract all of this out of the app and move it into a proper plugin).

ata-nas · 2026-05-07T14:42:54Z

+
+    private void persistRangeSet(ConcurrentLongRangeSet rangeSet, Path filePath) {
+        try {
+            Files.createDirectories(filePath.getParent());


The dir creation feels like something that should happen only once on startup.

I blindly copied this pattern from the other persist methods. But you are right, we don't need this called every time. Moved it to initialization

I recv'd the same feedback and moved it to the load method. So if the file does not exist on load, make sure the directory is created.

Followed the same pattern as yours, @berryware

That is ok, not opposed to it, just making a note in case it is of some value.

ata-nas · 2026-05-07T14:46:32Z

    }

-    private void checkForApplicationStateUpdates() {
+    private void checkForTssDataStateUpdates() {


Why the rename?

This will conflict with RSA changes. RSA update checks were added to checkForApplicationStateUpdates()

I renamed some methods and variables that looked very general to me. But now, as RSA update was added, I renamed back the method as it was

ata-nas · 2026-05-07T14:46:49Z

    }

+    @Override
+    public void addBlockRange(LongRange blockRange, BlockRangeType blockRangeType) {


How do we later get this data?

In the task (that I created, LOL), the requirement was only to persist this data. I can add another sub-task for accessing the data

The "stored" ranged set should generally be always available at runtime to everyone. It must also be dynamically updated at runtime so we have all the latest.

This task (and its parent) were focused on obtaining the information about the stored and available blocks.

As far as I can understand from other comments here, we are not sure how the data should be made available to the plugins: is it via push updates by ASF, or it is an on demand pattern: a method that exposes the data.

So, as I commented elsewhere, this seems to be beyond the scope of this task.

We should probably use this in serverDetailStatus API call. Currently we have just what the historical block facility says as available. But perhaps we want stored and available.

As discussed with @ata-nas , I added a getter method to the app state facility interface that returns the store block ranges

I think that adding a get method might be risky.
We've very strongly discouraged query/accessor methods in the App and facility interfaces because they are nightmarish when we remove these special facility interfaces from the App.
I would suggest an update method similar to how we update Context, but I also agree that making the data available to other plugins is not properly part of this Issue/PR.

Removed the accessor method

I think there is a slight misunderstanding, that might be on me (maybe I did not ask the question good enough). I mean how do we get the ranged sets themselves, not the block (or accessors for them). An update to the context will be consistent with the current setup, but thinking more about the future, adding a getter for the ranged sets (what's inside them) could also be an option.

Now, if we rely on the onContextUpdate call, we will be able to easily detect changes, so that might be the correct way to go.

There's already a task to dig deeper into this: #2852

ata-nas · 2026-05-08T09:44:30Z

+    final ConcurrentLongRangeSet storedBlocks = new ConcurrentLongRangeSet();
+
+    /** Blocks reported as available by BlockProviderPlugin implementations */
+    final ConcurrentLongRangeSet availableBlocks = new ConcurrentLongRangeSet();


Not sure about the addition of available blocks here. Maybe this is the right choice, but availability is per provider and not a consolidated result.

If 2 providers have block 5, then one deletes block 5, how do we update this? We cannot remove block 5 from here, cause it is still available, but also, we cannot find who has the block available. I think the historical block facility is the right place to keep this data. It holds a combined ranged set of all available blocks, valid for all providers, but we are also able to do non-destructive updates for individual ranged set there. If we chose to consolidate and persist this data, we can easily do so, but I think we can only ever reasonably load this data in memory by doing the scans we do for each provider. I feel like availability is more of a runtime thing.

As discussed with you earlier, we don't aim to replace the historical block facility. So we don't keep per-block provider data.

I am also not sure about the reason why we keep track of available blocks anyway. I just got this from discussion with @jsync-swirlds:

The block providers should be updating available blocks exclusively. The ASF should add available blocks to both range sets (stored and available), and stored blocks only to the stored blocks set.

The ASF should be storing both available and stored ranges to disk every so often (configurable based on blocks-as-timer, perhaps 1000 blocks).

Stored == Blocks that are known to be securely stored by this block node; mostly used to inform CN and backfill.
Available == Blocks that can be provided to a subscriber (which should generally be a subset of Stored).

If a block is available, it is assumed to be stored, while a stored block may, or may not, be available.

The reasoning for storing both available and stored on disk is twofold:

We should be able to report stored blocks at startup immediately, even if we update that shortly thereafter.

We don't currently track "deleted" blocks in a meaningful sense, we should probably work on making that sane and rational when we migrate from "special facilities" to plugin-to-plugin interactions.

The thing is that the available blocks, as data already exists in the Historical Block Facility. It's there where block provider plugins make updates to their available blocks ranged sets and the facility combines these. For available blocks, that is our source of truth. So I think it is there, where we must get our data for available blocks from. We can persist it, sure, but also, we must think about how each plugin, having an individual set, will notify the application state facility of that update. That is the tricky part. When a plugin adds/removes from its ranged set, that update is invisible to the application state facility as of today. The historical block facility is not reactive, but when called, it will look for registered sets in the combined ranged set.

So my concern is that we have to make these updates visible.

Nana-EC

A few minor items to correct, hopefully shouldn't take to long as I know this is blocking another item.
Feel free to triage and note which can be addressed in a follow up.

Another consideration is pull out the ApplicationStateFacility interface update and the BlockNodeContext update (which is missing from here) to it's own PR. That will allow other plugins to see it and you could work on this and the cloud storage improvement in parallel

Nana-EC · 2026-05-09T02:32:19Z

 * @param updateInitialDelay The amount of milliseconds that the {@code ApplicationStateFacility} waits before
 *     starting the scan interval.
 */
+// spotless:off


Let's start dropping these spotless flips

I thought that the agreement was to have them?

Anyway, I removed them.

We require the spotless disables in config or spotless mangles the file into something unreadable.
The team agreed that we'd prefer a clean and readable config files over slavish spotless adherence.

I restored the spotless off and on

Nana-EC · 2026-05-09T02:34:51Z


-        @Loggable @ConfigProperty(defaultValue = "500") @Min(100)
-        long updateScanInterval,
+        @Loggable @ConfigProperty(defaultValue = "/opt/hiero/block-node/node/stored-blocks-data.bin")


@berryware suggested we try to conform to JSON to make it easier for node operators to edit as needed

Changed to JSON, also added to protobuf. With JSON I can use one structure for both ranged sets

Nana-EC · 2026-05-09T02:40:45Z

            }
        }
+
+        try {


Seems odd place to create the dir.
Flows above check for file first and then I believe create the dir either at init() or before persistence.

Nana-EC · 2026-05-09T02:44:52Z

+        final long blocksToAdd = blockRange.size();
+        final long before = storedBlockCount.getAndAdd(blocksToAdd);
+        if (before / BLOCK_RANGE_PERSIST_INTERVAL < (before + blocksToAdd) / BLOCK_RANGE_PERSIST_INTERVAL) {
+            persistBlockRanges();


I believe the convention is to update a cached value and leave it to checkForApplicationStateUpdates() to handle persistence.
What you could to is have it call persistBlockRanges() and in persistBlockRanges() have the check for the interval of blocks stored if needed

Moved the persistence to checkForApplicationStateUpdates().

I left the check where it is (around the method call and not inside it) for two reasons:

The behavior is the same in the other two values which we persist

We call the persist method also in the stop method of the block node app and there we want to store it unconditionally

Nana-EC · 2026-05-09T02:45:32Z

                Thread.currentThread().interrupt();
            }
        }
+        persistBlockRanges();


Interesting, wondering if this should be done for all state data?
@berryware 🤔

@nana, so only persist in stopApplicationStateFacility()? Currently we store when the data changes. In the event of an unplanned system exit, we will have the latest changes.

@berryware I mean in addition.
So persistence would happen on changes and then on stop.
The need for stop is to to ensure that any in memory changes are captured.
In some cases we may find changes are fine in memory for some time and we periodically persist.
Something to consider in the design and apply if and when applicable

jsync-swirlds · 2026-05-18T23:49:31Z

    }

+    @Override
+    public void addBlockRange(LongRange blockRange, BlockRangeType blockRangeType) {


FYI, I'm not sure why we chose to add an enum here rather than just having a second method for stored versus available.
It's not a bad choice, but seems slightly over-engineered. I do not believe we'll ever add more block "range type"s beyond the current two...

Introduced two add methods and got rid of the enum

jsync-swirlds · 2026-05-18T23:53:40Z

+    default BlockRangeSet storedBlocks() {
+        return BlockRangeSet.EMPTY;
+    }


I do not think defaulting this is ideal.
It would be better to have the compiler feedback when we forget to implement the method.

The method is removed altogether

jsync-swirlds · 2026-05-18T23:56:59Z

+/**
+ * Persisted state of the stored and available block range sets.<br/>
+ * Both fields are persisted together in a single file so the two sets
+ * remain consistent across restarts.
+ */
+message BlockRangesState {
+    /**
+     * All block ranges that have been stored by this node.
+     */
+    repeated BlockRange stored_blocks = 1;
+    /**
+     * The subset of stored blocks that are currently available for retrieval.
+     */
+    repeated BlockRange available_blocks = 2;
+}
+


This does not belong in the block node API.
The BlockRangesState should be a new file in the internal protos so we do not pollute public API and are not constrained to the public API rules for this internal data structure.

Introduced a new internal proto file

ata-nas

Cleanup suggestions and a few clarifications.

ata-nas · 2026-05-20T10:57:52Z

+    @Override
+    public void addStoredBlockRange(LongRange blockRange) {
+        storedBlocks.add(blockRange);
+        storedBlockCount.addAndGet(blockRange.size());


I think the storedBlockCount must be the same as the storedBlocks.size()

Right, I removed the storedBlockCount field

ata-nas · 2026-05-20T11:18:27Z

+    final ConcurrentLongRangeSet storedBlocks = new ConcurrentLongRangeSet();
+
+    /** Blocks reported as available by BlockProviderPlugin implementations */
+    final ConcurrentLongRangeSet availableBlocks = new ConcurrentLongRangeSet();


The thing is that the available blocks, as data already exists in the Historical Block Facility. It's there where block provider plugins make updates to their available blocks ranged sets and the facility combines these. For available blocks, that is our source of truth. So I think it is there, where we must get our data for available blocks from. We can persist it, sure, but also, we must think about how each plugin, having an individual set, will notify the application state facility of that update. That is the tricky part. When a plugin adds/removes from its ranged set, that update is invisible to the application state facility as of today. The historical block facility is not reactive, but when called, it will look for registered sets in the combined ranged set.

So my concern is that we have to make these updates visible.

ata-nas · 2026-05-20T11:19:03Z

+    private final AtomicLong storedBlockCount = new AtomicLong(0);
+
+    /** Block count at the time of the last scheduled persist; only read/written by the scanner thread */
+    private long lastPersistedBlockCount = 0;


Does this need to be volatile? If really only read by the same thread, then we should be fine.

This is only read and written inside the checkForApplicationStateUpdates method, which in turns is called inside the scheduleAtFixedRate callback, which runs on the single-threaded applicationStateExecutor.

ata-nas · 2026-05-20T11:19:41Z

+        storedBlocks.add(blockRange);
+        availableBlocks.add(blockRange);
+        storedBlockCount.addAndGet(blockRange.size());


This method could call the addStoredBlockRange.

You are right, changed that

ata-nas · 2026-05-20T11:22:30Z

+            Files.write(tmp, json.toByteArray());
+            Files.deleteIfExists(filePath);
+            Files.createLink(filePath, tmp);
+            Files.deleteIfExists(tmp);


This seems a bit fragile. We want to overwrite the data, not delete it and then create it anew. I think we can be more careful about this. Possibly making a new file with the latest stored block as name and then deleting the old one. On startup, we look for the most recent one in case 2 or more and deleting the rest.

I copied this approach from the block files historic plugin, because we spent there a lot of time discussing what is the safest way to overwrite a file with another one

Just worried about some fragility here. If we are ok, we can proceed. Not sure where we copy from, seem to be missing something.

A few lines above we create the tmp file (a pattern from other persist methods in this class):

final Path tmp = filePath.resolveSibling(filePath.getFileName() + ".tmp");

Then we write the ranges in the tmp file:

final Bytes json = BlockRangesState.JSON.toBytes(toBlockRangesState()); Files.write(tmp, json.toByteArray());

And finally we delete the original file, create a link to it from the tmp file and delete the tmp file (a pattern that I took from the historic file plugin, where we discussed it back and forth before we decided that this is the way):

Files.deleteIfExists(filePath); Files.createLink(filePath, tmp); Files.deleteIfExists(tmp);

ata-nas · 2026-05-20T11:23:01Z

+            Files.deleteIfExists(filePath);
+            Files.createLink(filePath, tmp);
+            Files.deleteIfExists(tmp);
+            LOGGER.log(INFO, "Persisted block ranges to file: {0}", filePath);


Not sure of the value of this log message. But it is also infrequent, so should not be disruptive.

I followed the pattern from other persistence methods in the ASF

…able blocks Signed-off-by: Ivan St. Ivanov <[email protected]>

Signed-off-by: Ivan St. Ivanov <[email protected]>

…values managed by ASF Signed-off-by: Ivan St. Ivanov <[email protected]>

Signed-off-by: Ivan St. Ivanov <[email protected]>

codecov · 2026-05-21T04:32:30Z

Codecov Report

❌ Patch coverage is 89.47368% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...in/java/org/hiero/block/node/app/BlockNodeApp.java	89.47%	6 Missing ⚠️

@@             Coverage Diff              @@
##               main    #2738      +/-   ##
============================================
+ Coverage     81.47%   81.53%   +0.05%     
- Complexity     1559     1569      +10     
============================================
  Files           144      144              
  Lines          7413     7467      +54     
  Branches        777      779       +2     
============================================
+ Hits           6040     6088      +48     
- Misses         1050     1056       +6     
  Partials        323      323

Files with missing lines	Coverage Δ	Complexity Δ
.../node/app/config/state/ApplicationStateConfig.java	`100.00% <ø> (ø)`	`1.00 <0.00> (ø)`
...in/java/org/hiero/block/node/app/BlockNodeApp.java	`85.52% <89.47%> (+0.56%)`	`60.00 <8.00> (+10.00)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ivannov added this to the 0.34.0 milestone May 7, 2026

ivannov requested review from AlfredoG87, Nana-EC, ata-nas, berryware, jsync-swirlds and rockysingh May 7, 2026 14:15

ivannov self-assigned this May 7, 2026

ivannov requested review from a team as code owners May 7, 2026 14:15

ivannov added Improvement Code changes driven by non business requirements Block Node Issues/PR related to the Block Node. labels May 7, 2026

ata-nas reviewed May 7, 2026

View reviewed changes

ivannov force-pushed the 2702-asf-stored-blocks branch 2 times, most recently from 9f91d42 to beb9444 Compare May 8, 2026 08:30

ata-nas reviewed May 8, 2026

View reviewed changes

Nana-EC reviewed May 9, 2026

View reviewed changes

ivannov force-pushed the 2702-asf-stored-blocks branch 3 times, most recently from 0263ec1 to b86ed6c Compare May 14, 2026 09:32

jsync-swirlds reviewed May 18, 2026

View reviewed changes

Comment thread block-node/app/src/main/java/org/hiero/block/node/app/BlockNodeApp.java

jsync-swirlds reviewed May 18, 2026

View reviewed changes

ivannov force-pushed the 2702-asf-stored-blocks branch 2 times, most recently from f85d1c4 to b88deda Compare May 19, 2026 11:07

ata-nas reviewed May 20, 2026

View reviewed changes

ivannov added 2 commits May 20, 2026 15:47

2702 Add method to Application State Facility to add stored and avail…

c714d52

…able blocks Signed-off-by: Ivan St. Ivanov <[email protected]>

Empty implementation of addBlockRange in PluginTestBase

4ff4238

Signed-off-by: Ivan St. Ivanov <[email protected]>

ivannov added 6 commits May 20, 2026 15:47

Synchronize access to storing stored blocks data

d56b1d5

Signed-off-by: Ivan St. Ivanov <[email protected]>

Persist the stored and available blocks as JSON along with the other …

f7bf7b2

…values managed by ASF Signed-off-by: Ivan St. Ivanov <[email protected]>

Add getter method to the interface to obtain the stored block ranges

fdc1179

Signed-off-by: Ivan St. Ivanov <[email protected]>

Extract ranges file directory creation in load method

bc7edd4

Signed-off-by: Ivan St. Ivanov <[email protected]>

Reworked ASF interface with respect to stored and available blocks

3e1a832

Signed-off-by: Ivan St. Ivanov <[email protected]>

Simplified stored blocks management

cb44e19

Signed-off-by: Ivan St. Ivanov <[email protected]>

ivannov force-pushed the 2702-asf-stored-blocks branch from 3461d03 to cb44e19 Compare May 20, 2026 12:47

Nana-EC requested review from ata-nas and jsync-swirlds May 21, 2026 04:31

                           }
                       }
+                      try {

Conversation

ivannov commented May 7, 2026

Reviewer Notes

Related Issue(s)

Uh oh!

codacy-production Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

ata-nas May 20, 2026 • 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!

jsync-swirlds May 18, 2026 • 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!

Nana-EC left a comment

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!

codacy-production Bot commented May 7, 2026 •

edited

Loading

ata-nas May 20, 2026 •

edited

Loading

jsync-swirlds May 18, 2026 •

edited

Loading

Nana-EC May 18, 2026 •

edited

Loading