Skip to content

Backport 6.x: On recovery, set UVM descriptor SVN to min(existing, new)#7730

Merged
achamayou merged 4 commits intorelease/6.xfrom
copilot/backport-pr-7717-to-release-6-x
Mar 10, 2026
Merged

Backport 6.x: On recovery, set UVM descriptor SVN to min(existing, new)#7730
achamayou merged 4 commits intorelease/6.xfrom
copilot/backport-pr-7717-to-release-6-x

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 9, 2026

  • CHANGELOG.md: Add entry about UVM descriptor SVN minimum fix
  • CMakeLists.txt: Add internal_tables_access_test unit test
  • src/node/uvm_endorsements.h: Add parse_svn declaration
  • src/node/uvm_endorsements.cpp: Extract parse_svn utility function
  • src/service/internal_tables_access.h: Add recovering param to trust_node_uvm_endorsements
  • src/node/rpc/node_frontend.h: Pass recovering to trust_node_uvm_endorsements
  • src/node/rpc/test/internal_tables_access_test.cpp: Custom main() with openssl_sha256_init/shutdown
  • tests/e2e_operations.py: Update run_initial_uvm_descriptor_checks to bump SVN before recovery
Original prompt

Backport of PR #7717 to release/6.x

This is a backport of #7717 (merged into main on 2026-03-06, merge commit 5a41a9519f2aa1d86fbda85191dc262e97ed3f46).

Original PR Description

During recovery, the node was unconditionally overwriting the UVM descriptor SVN with the value from the new node's endorsements. This could raise the SVN above what was previously stored, which is incorrect — recovery should never increase the SVN.

Changes to backport

The following files were changed in #7717 and need to be cherry-picked / applied to the release/6.x branch:

  1. CHANGELOG.md — Add changelog entry: "On recovery, the UVM descriptor SVN is now set to the minimum of the previously stored value in the KV and the value found in the new node's startup endorsements. On start, the behaviour is unchanged (On Recovery, the node must set the UVM descriptor SVN to the minimum of the current value in the KV and the one found in startup endorsements #7716)."

  2. CMakeLists.txt — Add the new internal_tables_access_test unit test:

    add_unit_test(
      internal_tables_access_test
      ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/internal_tables_access_test.cpp
      ${CCF_DIR}/src/node/uvm_endorsements.cpp
    )
    target_link_libraries(internal_tables_access_test PRIVATE ccfcrypto ccf_kv)
  3. src/node/uvm_endorsements.h — Add declaration for the new parse_svn utility function:

    size_t parse_svn(const std::string& svn_str);
  4. src/node/uvm_endorsements.cpp — Extract a parse_svn utility function that parses an SVN string to size_t, and refactor matches_uvm_roots_of_trust to use it instead of inline std::from_chars calls. The new function:

    size_t parse_svn(const std::string& svn_str)
    {
      size_t svn = 0;
      auto result =
        std::from_chars(svn_str.data(), svn_str.data() + svn_str.size(), svn);
      if (result.ec != std::errc())
      {
        throw std::runtime_error(
          fmt::format("Unable to parse svn value {} to unsigned", svn_str));
      }
      return svn;
    }
  5. src/service/internal_tables_access.htrust_node_uvm_endorsements now accepts a bool recovering parameter. When recovering is true, it reads the existing SVN from the KV for the same DID/feed and writes min(existing, new) using parse_svn. The feed entry is merged into the existing map for the DID, preserving other feeds rather than overwriting the entire map. A changed boolean tracks whether the map was actually modified. The KV write is skipped when nothing changed. When recovering is false, the original behavior is preserved.

  6. src/node/rpc/node_frontend.h — Pass recovering (already computed in the /create handler) to trust_node_uvm_endorsements:

    InternalTablesAccess::trust_node_uvm_endorsements(
      ctx.tx, in.snp_uvm_endorsements, recovering);
  7. src/node/rpc/test/internal_tables_access_test.cpp — New unit test file with 5 test cases covering:

    • Not recovering, empty map
    • Recovering, new DID not in map
    • Recovering, existing DID, new feed
    • Recovering, existing DID and feed, lower SVN (should update to lower)
    • Recovering, existing DID and feed, higher SVN (should keep existing lower value)
  8. tests/e2e_operations.py — In run_initial_uvm_descriptor_checks, after starting the network, fetch the current join policy to get UVM endorsement values, bump the SVN by 10, and propose the bumped UVM endorsement before stopping the network. This ensures recovery will encounter a higher SVN and must correctly use the minimum.

Instructions

  • The base branch for this PR must be release/6.x.
  • Try to cherry-pick the merge commit 5a41a9519f2aa1d86fbda85191dc262e97ed3f46 first. If that doesn't apply cleanly, manually apply the equivalent changes to each file listed above, adapting to any differences in the release/6.x branch.
  • The CHANGELOG.md entry should go under the appropriate section for the release branch (it may differ from main).
  • Make sure the code compiles and is consistent with the rest of the release/6.x codebase.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Backport #7717 into release/6.x branch as a separate PR

Assistant:

Let me first look at the details of PR #7717 to understand what needs to be backported.

Assistant: I need to find the repository for PR #7717. Let me search for it.

Assistant: I need to find which repository PR #7717 belongs to. Let me search for it.

Assistant: I need to find the repository that contains PR #7717. Let me look at your repositories to identify the correct one.

Assistant: I need to find which repository PR #7717 belongs to. Could you please tell me the repository (in owner/repo format) where PR #7717 exists? That way I can look it up and create the backport PR to the release/6.x branch.

User: CCF repository https://github.com/microsoft/CCF/

Assistant: Now I have all the details about PR #7717. Let me create the backport PR tar...

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits March 9, 2026 23:25
…existing KV value and startup endorsements

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Copilot AI changed the title [WIP] Backport changes from PR #7717 to release/6.x Backport #7717 to release/6.x: On recovery, set UVM descriptor SVN to min(existing, new) Mar 9, 2026
@maxtropets maxtropets marked this pull request as ready for review March 9, 2026 23:30
@maxtropets maxtropets requested a review from a team as a code owner March 9, 2026 23:30
@maxtropets maxtropets changed the title Backport #7717 to release/6.x: On recovery, set UVM descriptor SVN to min(existing, new) Backport 6.x: On recovery, set UVM descriptor SVN to min(existing, new) Mar 9, 2026
@maxtropets maxtropets added the 6.x-backport Highlight PRs targeting the 6.x branch label Mar 9, 2026
@maxtropets
Copy link
Copy Markdown
Collaborator

@copilot add ccf::crypto::openssl_sha256_init(); and shutdown to the internal_access test main()

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 9, 2026

@copilot add ccf::crypto::openssl_sha256_init(); and shutdown to the internal_access test main()

Done in 981987e. Changed DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN to DOCTEST_CONFIG_IMPLEMENT, added #include "crypto/openssl/hash.h", and added a custom main() matching the pattern used in node_frontend_test.cpp.

@achamayou achamayou merged commit de7b364 into release/6.x Mar 10, 2026
19 of 21 checks passed
@achamayou achamayou deleted the copilot/backport-pr-7717-to-release-6-x branch March 10, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.x-backport Highlight PRs targeting the 6.x branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants