DID Support #498
DID Support #498
Conversation
…e serialized type
… be base 10, not base 16. Also serialize XChainClaimIds as hex in JSON, and add a new XChainCount type that also serializes to hex in JSON
Add new transactions to SignatureUtils switches
Add attestations that can be signed. Add ability to sign attestations to signature services.
|
|
||
| @Override | ||
| public void serialize(DidData DidData, JsonGenerator gen, SerializerProvider provider) throws IOException { | ||
| gen.writeString(DidData.value()); |
There was a problem hiding this comment.
Should this data actually be hex-encoded? The spec shows this as a string in JSON, but in rippled it's just a BLOB.
There was a problem hiding this comment.
It should be hex encoded, but I'm not currently checking that it is in DidData, DidDocument or DidUri. I could do that, but it adds an extra hex decode, and I've generally stopped adding validation to our models because rippled often breaks those validations. However, if a developer sets one of these fields to a non-hex String and tries to serialize it to sign, the binary codec will throw an error because it's expecting a hex string, so their errors will at least be caught before submitting to rippled.
There was a problem hiding this comment.
Yes, that's reasonable. I'm not too worried in this particular case because the input on line 44 is going to be coming from rippled, and it should be HEX.
On the constructors of these threee (DidData.of(...) it might be worth adding validation that only HEX can be supplied there -- but to you're point, the binary codec will throw. Maybe this relates to #499, and can be fixed there -- but the binary codec errors aren't very helpful.
This isn't a blocker for me here, so if you don't want to do anything for now, I'm good with that (in which case, please "resolve" this comment).
xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/DidUriDeserializer.java
Show resolved
Hide resolved
xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/DidUriSerializer.java
Show resolved
Hide resolved
| .lastLedgerSequence(sourceAccountInfo.ledgerIndexSafe().unsignedIntegerValue().plus(UnsignedInteger.valueOf(4))) | ||
| .signingPublicKey(sourceKeyPair.publicKey()) | ||
| .data(DidData.of("617474657374")) | ||
| .uri(DidUri.of("6469645F6578616D706C65")) |
There was a problem hiding this comment.
What does rippled do if this isn't HEX?
There was a problem hiding this comment.
See comment above -- I can't actually submit a transaction with a non-hex value for any of these fields because the binary codec will throw an exception.
You could talk me into validating that DidData, DidUri, and DidDocument are hex encoded in those objects so developers get better exceptions...
There was a problem hiding this comment.
Maybe make a comment in #499 to improve the binary codec errors.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #498 +/- ##
============================================
+ Coverage 91.52% 91.61% +0.08%
- Complexity 1748 1774 +26
============================================
Files 355 365 +10
Lines 4884 4948 +64
Branches 403 408 +5
============================================
+ Hits 4470 4533 +63
- Misses 281 282 +1
Partials 133 133 ☔ View full report in Codecov by Sentry. |
Fixes #496. Adds XLS-40d DID support.
Includes new transactions:
DidSetDidDeleteIncludes new ledger enty:
DidObject