Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds rp to exn to work with >1.2.0-dev4 #905

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

m00sey
Copy link
Member

@m00sey m00sey commented Dec 13, 2024

No description provided.

@m00sey m00sey requested a review from pfeairheller December 13, 2024 19:12
Copy link
Contributor

@daidoji daidoji left a comment

Choose a reason for hiding this comment

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

What is "newer KERI"?

@@ -304,7 +304,7 @@ class Serder:
Ilks.bar: Fieldage(saids={Saids.d: DigDex.Blake3_256},
alls=dict(v='', t='',d='', i='', dt='', r='',a=[])),
Ilks.exn: Fieldage(saids={Saids.d: DigDex.Blake3_256},
alls=dict(v='', t='', d='', i="", p="", dt='', r='', q={},
alls=dict(v='', t='', d='', i="", rp="", p="", dt='', r='', q={},
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the RP field?

Copy link
Member Author

@m00sey m00sey Dec 13, 2024

Choose a reason for hiding this comment

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

The PR is against 1.1 - renamed to be clear.

Rp field was added 1.2.0-dev4

Copy link
Contributor

Choose a reason for hiding this comment

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

The PR is against 1.1 - renamed to be clear.

Rp field was added 1.2.0-dev4

Sure, but what is it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Top level recipient field. This was discussed some time ago, hence its addition to 1.2.+
It was added to save unpacking the a.r field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Top level recipient field. This was discussed some time ago, hence its addition to 1.2.+ It was added to save unpacking the a.r field.

Do you remember when? It hasn't been added to the specs yet.

Copy link
Member Author

@m00sey m00sey Dec 14, 2024

Choose a reason for hiding this comment

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

The specs show Ver_2_0 but it looks like it’s ri in there, these are against Ver_1_0 and Ver_1_1

@m00sey m00sey changed the title adds rp to exn to work with newer witnesses running on newer keri adds rp to exn to work with >1.2.0-dev4 Dec 13, 2024
@m00sey m00sey merged commit a700ce6 into v1.1.27 Dec 13, 2024
12 checks passed
@m00sey m00sey deleted the bug/fix-exn-compatability branch December 13, 2024 21:06
@SmithSamuelM
Copy link
Collaborator

The fix should be that an ri field for receiver identifier (AID) be added to the EXN and XIP messages in version 2. This is what is in the specification for KERI. The reason for ri and not rp is that the convention when the field value is an AID is for the field label to either be i or if there is already and i to use a two character field label where the second character is i and the first character is the modifier. For example di is the delegator identifier field.

rp was put in at some point in the current code before we had the discussion of using the convention.
If you are going to update version 1 of keri, then I suggest replacing the rp with the ri field label. Then it would already be version 2 compliant.

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.

4 participants