Merge `release/anemone-v1.1.3` into `develop` #922

dhedey · 2024-04-23T19:14:31Z

Important

Please read our Contributing Guidelines before opening a PR.
Before creating your PR, please ensure you have used the correct base branch as per the branching strategy, both for branching from, and in the PR UI above.
- For most code changes, this is develop.
- For stand-alone docs changes, this is main.
- For workflow changes, this is the oldest supported release/* branch.
Please remove these sections as you fill out your PR.

Summary

Tip

Start with the context of your PR. Why are you making this change? What does it address? Link back to an issue if relevant.

Then summarise the changes that were made. Bullet points are fine.

Details

Tip

This section is optional. Go into more detail about the changes that were made, or the thinking behind the changes.

Testing

Tip

Explain what testing / verification is done, including manual testing or automated testing.

… into feature/browse_object_field

…ylon-node into feature/browse_entity_iterator

… into feature/browse_object_field

…ylon-node into feature/browse_entity_iterator

…or Node).

This goes into `/feature/browse_api` long-lived branch. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#4.-Object-Field-Lookup

⚠️ This builds on top of #733 (re-uses some code) and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#1.-Entity-Listing but the main focus of the PR is on re-usable listing-with-pages-on-continuation-tokens infra (please see the heavily-documented abstractions introduced in `paging.rs`).

⚠️ This builds on top of #736 (refactors some stuff introduced there, in order to re-use it here) and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#2.-Entity-Information .

⚠️ This builds on top of #739 (doesn't strictly depend on anything, but touches some files due to small refactors) and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#5.-Object-Collection-Listing . ❗ **NOTE:** This PR contains a temporary provisional "faux paging" impl! We need to add support for "iterate from" to the Engine's `SubstateDatabase` - afterwards, we will super-easily migrate this to proper paging (literally `+1 -4` PR)

⚠️ This builds on top of #741 (uses the same mechanisms) and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#7.-KVStore-Collection-Listing . ❗ **NOTE:** (same as in #741) This PR contains a temporary provisional "faux paging" impl! We need to add support for "iterate from" to the Engine's `SubstateDatabase` (already pending at radixdlt/radixdlt-scrypto#1648) - afterwards, we will super-easily migrate this to proper paging (literally `+1 -4` PR)

⚠️ This builds on top of #743 and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#3.-Blueprint-Details .

⚠️ This builds on top of #744 and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This is in the "Browse API" PR-chain, but actually it is a pretty generic feature: - Implemented Programmatic JSON **deser**ializer - We need it now (yeah for Browse API), and Engine does not offer one (they only serialize) - The impl is stolen from RET (and then heavily adjusted for Node's specifics; but not beyond recognition) - The plan is to either move this impl to Engine (or implement the same on the Engine's side), and we'll then be able to remove this code from Node (and maybe even from RET?) - created the Programmatic JSON **ser**ializer facade - Just so that it can live next to the **deser**ializer - ofc it is only facading the Engine's serialization impl - it is used to test the **deser**ializer for compatibility with Engine's **ser**ializer (the "round-trip codec test") Note: The **ser**ializer facade is used right away, but the **deser**ializer is currently unused (and rightfully produces warnings) [except in tests]; it will soon be used by Browse API's "lookup kv store/collection" endpoints.

⚠️ This builds on top of #747 (relies on the programmatic JSON deserialization support introduced there) and ultimately wants to merge into `feature/browse_api` _(not `develop`!)_. This implements [https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#3.-Blueprint-Details .](https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#8.-KVStore-Collection-Lookup) . (with all the infra already provided by the previous Browse API PRs, this one is actually trivial) (the only other remaining endpoint, `/browse/object/collection/entry`, should be trivial based on this one)

…re/browse_api

Just to keep the long-lived branch up-to-date.

… into feature/browse_object_collection_entry

This goes into `/feature/browse_api` long-lived branch. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#6.-Object-Collection-Lookup 🎉 This is the last endpoint in the first slice of Browse API 🎉 (there will still be 2-3 PRs fixing some found bugs, including in Scrypto repo)

…re/browse_api

…ording to conditions).

## Summary Addresses item 4. of https://radixdlt.atlassian.net/browse/NODE-633, as in the title. ## Testing N/A, it is only docs change.

…g persistence).

## Summary As discussed at https://rdxworks.slack.com/archives/C01M90Y2448/p1713868409449979?thread_ts=1713866025.075899&cid=C01M90Y2448 ## Details This change also renames a string marker in DB, which means that a ledger wipe will be needed if this branch was deployed anywhere before. ## Testing Just a rename, regression tests have to pass.

## Summary Addresses a build failure spotted at https://github.com/radixdlt/babylon-node/actions/runs/8797900181/job/24143785471#step:19:527. We recently had to bump a version, but it is not available for ARM (see https://packages.debian.org/bookworm/openjdk-17-jre-headless). ## Testing N/A, a build problem only.

github-actions · 2024-04-23T19:43:42Z

Docker tags
docker.io/radixdlt/private-babylon-node:pr-922
docker.io/radixdlt/private-babylon-node:41b7015ab4
docker.io/radixdlt/private-babylon-node:sha-41b7015

dhedey · 2024-04-25T14:21:16Z

core-rust/state-manager/src/store/historical_state.rs

    at_state_version: StateVersion,
+
+    // Note: "Why do we even need to capture `'t`?"


Well, this is because of the fact that e.g. the SubstateTier's into_iter_substate_summaries_from method exposes an iterator with a lifetime of the underlying store rather than the entity tier (in other words, it can outlive the tier).

However, the underlying store comes from self.base_store.deref() and we need someone to capture the constraint that the reference created lives longer than the &self parameter in order to satisfy the SubstateDatabase trait definition for list_entries_from.

But self.base_store.deref() could either resolve to some random lifetime if base_store is a reference, or could resolve to &self.base_store, so could be bounded by &self.

We need some way of capturing that "the lifetime of self.base_store.deref()" outlives ANY reference to self. This is actually extra information -- and this phantom lifetime is the best way of doing that.

Perhaps it'd be clearer to actually make it a PhantomData<&'s S> and have 's, S: 's, DS: Deref<Target = S>.

Or just PhantomData<&'s ()>. I think PhantomData<&'s DS> is a little misleading - because it's not pretending to encompass a reference to DS, but actually to some underlying store.

dhedey · 2024-04-25T14:22:58Z

core-rust/state-manager/src/store/historical_state.rs

+    }
+}
+
+impl<'s, R: ReadableRocks + 's, DS: Deref<Target = StateManagerDatabase<R>>> ReNodeListingIndex


EntityListingIndex*

dhedey

Various comments on the index building logic. I don't think any of this is a release blocker, although we'd ideally fix the logging.

Perhaps we can raise a ticket to tidy up the rest?

dhedey · 2024-04-25T14:38:58Z

core-rust/state-manager/src/store/rocks_db.rs

+            );
+            if state_version.number() % TXN_FLUSH_INTERVAL == 0 || receipts_iter.peek().is_none() {
+                info!(
+                    "ReNode listing indices updated to {}; flushing...",


That's also a lot! of info logs catching up on mainnet which has 75 million state versions right now.

Perhaps better to have info logs every 1 million transactions rather than every 10000? And then if you want logs every flush, perhaps make them debug! logs?

dhedey · 2024-04-25T15:05:12Z

core-rust/state-manager/src/store/traits.rs

+    use super::*;
+    use std::ops::Range;
+
+    pub trait ReNodeListingIndex {


EntityListingIndex*

dhedey · 2024-04-25T15:06:23Z

core-rust/state-manager/src/store/traits.rs

+    }
+
+    /// An Object's ID and its blueprint name.
+    /// This is a "technical" structure stored in one of the ReNode-listing indices.


dhedey · 2024-04-25T15:07:27Z

core-rust/state-manager/src/store/rocks_db.rs

@@ -764,6 +800,10 @@ impl ActualStateManagerDatabase {
        state_manager_database.restore_december_2023_lost_substates(network);
        state_manager_database.ensure_historical_substate_values();

+        if state_manager_database.config.enable_entity_listing_indices {


It would be more consistent to use an ensure_ style and have this config check internal to the method? To either set up or tear down the index.

We're also missing the teardown logic for this index.

dhedey · 2024-04-25T15:09:27Z

core-rust/state-manager/src/store/rocks_db.rs

+            .peekable();
+
+        while let Some((state_version, receipt)) = receipts_iter.next() {
+            self.batch_update_entity_listing_indices(


It's better to use the substate store than the receipt store for this IMO.

It's both faster, and doesn't require full receipt history to do.

Not worth doing now, but in future, maybe worth raising as a separate ticket to change how we rebuild this index?

Ohh actually I see that we have to use the transaction receipts, due to the way the index includes the state version and index in transaction bits! Okay, ignore this :)

I went through the same journey when implementing that :p

dhedey · 2024-04-25T15:11:00Z

core-rust/state-manager/src/store/rocks_db.rs

+                db_context.flush();
+            }
+        }
+        info!("ReNode listing indices are caught up.");


dhedey · 2024-04-25T15:14:38Z

core-rust/state-manager/src/store/rocks_db.rs

@@ -2154,6 +2315,57 @@ impl<R: ReadableRocks> IterableAccountChangeIndex for StateManagerDatabase<R> {
    }
 }

+impl<R: ReadableRocks> ReNodeListingIndex for StateManagerDatabase<R> {


EntityListingIndex*

dhedey · 2024-04-25T15:20:05Z

core-rust/state-manager/src/store/rocks_db.rs

+            .next()
+            .expect("next version");
+
+        let mut receipts_iter = db_context


Hmm I thought catchup_from_version is actually previous_indexed_up_to_version?
So shouldn't we start iterating from previous_indexed_up_to_version + 1 inclusive?

Although I imagine setting these is idempotent, so we won't have duplicated entries? So it might be safe?

a tiny .next() (in line 2132 above) is the + 1 you are looking for :D

FOR ONCE I avoided the "off by one" 🎉 (I think 🤔)

Ahh I thought that was a .next() to return the first value from an iterator!
In retrospect, perhaps next is a little too overloaded, but it's minor anyway?

In terms of this code, tiny suggestion - maybe separating out let catchup_from_version = previously_updated_to_version.next(); might make it clearer / more explicit for noobs like me?

this I can do: e8b08cc 👍

dhedey · 2024-04-25T15:23:07Z

core-rust/state-manager/src/store/rocks_db.rs

+            };
+            let SubstateChangeAction::Create { new } = type_info_creation else {
+                panic!(
+                    "type info substate should be immutable: {:?}",


This seems quite scary. e.g. this won't be true if the blueprint version is updated eventually.

But the entity id and blueprint name should be fixed by the system, so I think it's fine to just ignore updates.

dhedey · 2024-04-25T15:23:22Z

core-rust/state-manager/src/store/rocks_db.rs

+            };
+            let type_info = scrypto_decode::<TypeInfoSubstate>(new).expect("decode type info");
+
+            let entity_type = node_id.entity_type().expect("type of upserted ReNode");


… `delete_all()`).

## Summary Addresses https://radixdlt.atlassian.net/browse/NODE-638 (i.e. all the comments on #922 (review)) ## Testing Just regression needs to pass.

sonarqubecloud · 2024-04-26T11:51:44Z

Quality Gate passed

Issues
21 New issues
0 Accepted issues

Measures
0 Security Hotspots
18.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jakrawcz-rdx and others added 30 commits November 1, 2023 12:18

Impl of /browse/object/field endpoint.

e773479

Impl of /browse/entity/iterator endpoint (mostly: paging infra).

79eca98

Merge branch 'feature/browse_api' of github.com:radixdlt/babylon-node…

9545104

… into feature/browse_object_field

Merge branch 'feature/browse_object_field' of github.com:radixdlt/bab…

fc05c8a

…ylon-node into feature/browse_entity_iterator

Merge branch 'feature/browse_api' of github.com:radixdlt/babylon-node…

0545f9a

… into feature/browse_object_field

Merge branch 'feature/browse_object_field' of github.com:radixdlt/bab…

9560f2b

…ylon-node into feature/browse_entity_iterator

Impl of /browse/entity/info endpoint.

cb5a131

Preparing the paging infra: Added Result<> and Key support.

b70f28d

Impl of /browse/object/collection/iterator endpoint.

2cfdc16

High-level test for /browse/object/collection/iterator.

89271a4

Impl of /browse/kv_store/iterator endpoint.

354b2c7

Impl of /browse/blueprint/info endpoint.

fa01bf0

Impl for Programmatic JSON deserializer (copied from RET + adjusted f…

ec13634

…or Node).

Impl of /browse/kv_store/entry endpoint (KV-Store look-up).

93b5e25

Impl of /browse/object/field endpoint (#733)

f2224c5

This goes into `/feature/browse_api` long-lived branch. This implements https://radixdlt.atlassian.net/wiki/spaces/RPNV1/pages/3189211214/RNP-12+-+Browse+API+sub-API+within+Core+API#4.-Object-Field-Lookup

Merge branch 'develop' of github.com:radixdlt/babylon-node into featu…

a2b3e33

…re/browse_api

Impl of /browse/object/collection/entry endpoint.

08ff6ff

Merge develop into feature/browse_api (#754)

66f289d

Just to keep the long-lived branch up-to-date.

Merge branch 'feature/browse_api' of github.com:radixdlt/babylon-node…

0383b0a

… into feature/browse_object_collection_entry

Reverting development debug log line.

057490d

Merge branch 'develop' of github.com:radixdlt/babylon-node into featu…

667eaf3

…re/browse_api

Fix: Only return object instance's fields that are present on it (acc…

6f13326

…ording to conditions).

les-sheppard and others added 10 commits April 22, 2024 22:17

remove whitespace from add-artifacts-to-release worflow

e77415b

Describe Engine State API's "beta" status in the apidocs (#917)

6ddda37

## Summary Addresses item 4. of https://radixdlt.atlassian.net/browse/NODE-633, as in the title. ## Testing N/A, it is only docs change.

Incorrect version in gradle task action (#918)

afd5611

Merge branch 'feature/browse_api' into release/anemone-v1.1.3

938fcca

Pin a different version of openjdk17 deps in docker for ARM.

f767e43

Fix the missed $ for docker arg.

284afb6

Fix the missed ARG after another docker stage.

cfa0066

Rename the "RE Node listing" to "Entity listing" everywhere (includin…

aa9289d

…g persistence).

dhedey commented Apr 25, 2024

View reviewed changes

jakrawcz-rdx added 5 commits April 25, 2024 18:11

Less frequent logging on Entity-listing indices catch-up.

74cc106

Rename the missed occurrences of ReNode to Entity.

0075891

Delete Entity-listing indices when disabled.

6036627

Ignore (instead of panic) on TypeInfo update.

3b4353f

Add missed BoundedDbCodec impls for Entity-listing codecs (for fast…

55e7412

… `delete_all()`).

jakrawcz-rdx mentioned this pull request Apr 25, 2024

Entity-listing indices tweaks #926

Merged

jakrawcz-rdx and others added 2 commits April 26, 2024 12:04

More readable "catch-up from version" calculation.

e8b08cc

Entity-listing indices tweaks (#926)

a843e2c

## Summary Addresses https://radixdlt.atlassian.net/browse/NODE-638 (i.e. all the comments on #922 (review)) ## Testing Just regression needs to pass.

iamyulong temporarily deployed to publish-artifacts April 26, 2024 10:46 — with GitHub Actions Inactive

fix: Update for v1.1.3.1

dc9f981

iamyulong temporarily deployed to publish-artifacts April 26, 2024 11:28 — with GitHub Actions Inactive

jakrawcz-rdx marked this pull request as ready for review April 29, 2024 14:32

jakrawcz-rdx self-requested a review April 29, 2024 14:32

jakrawcz-rdx approved these changes Apr 29, 2024

View reviewed changes

jakrawcz-rdx merged commit 3809068 into develop Apr 29, 2024
68 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge `release/anemone-v1.1.3` into `develop` #922

Merge `release/anemone-v1.1.3` into `develop` #922

dhedey commented Apr 23, 2024

github-actions bot commented Apr 23, 2024 •

edited

Loading

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey left a comment

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

jakrawcz-rdx Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

jakrawcz-rdx Apr 25, 2024

dhedey Apr 26, 2024

jakrawcz-rdx Apr 26, 2024

dhedey Apr 25, 2024

dhedey Apr 25, 2024

sonarqubecloud bot commented Apr 26, 2024

		at_state_version: StateVersion,

		// Note: "Why do we even need to capture `'t`?"

Merge release/anemone-v1.1.3 into develop #922

Merge release/anemone-v1.1.3 into develop #922

Conversation

dhedey commented Apr 23, 2024

Summary

Details

Testing

github-actions bot commented Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhedey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 26, 2024

Quality Gate passed

Merge `release/anemone-v1.1.3` into `develop` #922

Merge `release/anemone-v1.1.3` into `develop` #922

github-actions bot commented Apr 23, 2024 •

edited

Loading