-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: support hashed content CIP-30 data content which is typically g… #593
base: develop
Are you sure you want to change the base?
Conversation
@@ -78,7 +79,7 @@ dependencies { | |||
runtimeOnly("org.postgresql:postgresql") | |||
|
|||
implementation("org.cardanofoundation:merkle-tree-java:0.0.7") | |||
implementation("org.cardanofoundation:cip30-data-signature-parser:0.0.11") | |||
implementation("org.cardanofoundation:cip30-data-signature-parser:0.0.12-SNAPSHOT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
change when released 0.0.12 to maven central
a71479f
to
0b870a5
Compare
@@ -28,6 +28,7 @@ configurations { | |||
} | |||
|
|||
repositories { | |||
mavenLocal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO remove when cip30-data-signature-parser:0.0.12-SNAPSHOT is released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually safe to keep it as well, right?
@@ -60,7 +60,7 @@ public class Vote extends AbstractTimestampEntity { | |||
private String signature; | |||
|
|||
@Column(name = "payload", nullable = false, columnDefinition = "text", length = 2048) | |||
@Nullable | |||
@Nullable // TODO remove nullable since payload is now always required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in future releases, don't wanna do db migration now, too risky and not necessary.
0b870a5
to
bf8f892
Compare
bf8f892
to
8a73123
Compare
…enerated by hardware wallets.
8a73123
to
372bc34
Compare
@@ -122,7 +125,37 @@ protected void doFilterInternal(HttpServletRequest req, | |||
|
|||
val walletId = maybeAddress.orElseThrow(); | |||
|
|||
val cipBody = cipVerificationResult.getMessage(MessageFormat.TEXT); | |||
var cipBody = cipVerificationResult.getMessage(MessageFormat.TEXT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because i saw this: a cose_sign1 signature/structure is still valid even without a payload embedded. not sure how often this is used, but the requesting process already know the payload, so there is no 100% need to have a payload included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and it works :)
@@ -28,6 +28,7 @@ configurations { | |||
} | |||
|
|||
repositories { | |||
mavenLocal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually safe to keep it as well, right?
…enerated by hardware wallets.
Many thanks to the OG Cardano Community member: Martin Lang (@gitmachtl) in helping to debug and find out the root cause.