Skip to content

Conversation

@Bronek
Copy link
Collaborator

@Bronek Bronek commented Oct 24, 2025

High Level Overview of Change

Fix object template for Credential object.

Context of Change

Field sfSubjectNode is not populated by CredentialCreate in self-issued credentials. Rather than fixup the Credentials already on the ledger, we can in this case safely change the object template for this field from soeREQUIRED to soeOPTIONAL.

We know that this is safe (despite no amendment) because:

  • object template is never a part of serialized data and never used in hashing or object signing
  • we have done this before, changing sfAuthorize in DepositPreauth in XLS-70d Credentials #5103

If we don't do it, the only safe way to refer to sfSubjectNode will be by comparing sfIssuer against sfSubject beforehand, as currently done in CredentialDelete transaction. Although in this specific context this is fine, such approach in general is unsafe.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

@Bronek Bronek requested a review from a team as a code owner October 24, 2025 10:51
@Bronek Bronek changed the title Change Credential sfSubjectNode to optional Change Credential sfSubjectNode from required to optional Oct 24, 2025
@Bronek Bronek added this to the 3.0.0 milestone Oct 24, 2025
@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.5%. Comparing base (2bf77cc) to head (887d525).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5936     +/-   ##
=========================================
- Coverage     79.5%   78.5%   -1.0%     
=========================================
  Files          817     817             
  Lines        72198   69017   -3181     
  Branches      8293    8274     -19     
=========================================
- Hits         57392   54199   -3193     
- Misses       14806   14818     +12     
Files with missing lines Coverage Δ
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <ø> (ø)

... and 492 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Bronek Bronek requested review from mvadari and oleks-rip October 24, 2025 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants