Skip to content

Commit d269b17

Browse files
committed
fix(cardano-blockchain-types): cip36 err report
Signed-off-by: bkioshn <[email protected]>
1 parent b403820 commit d269b17

File tree

3 files changed

+51
-45
lines changed

3 files changed

+51
-45
lines changed

rust/cardano-blockchain-types/src/metadata/cip36/key_registration.rs

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ pub(crate) struct Cip36KeyRegistration {
4949
pub nonce: Option<u64>,
5050
/// Registration Purpose (Always 0 for Catalyst).
5151
/// Field 5 in the CIP-36 61284 Spec.
52-
/// None if it is not set.
53-
pub purpose: Option<u64>,
52+
/// Default to 0.
53+
pub purpose: u64,
5454
/// Raw nonce (nonce that has not had slot correction applied).
5555
/// None if it is not set.
5656
pub raw_nonce: Option<u64>,
@@ -81,6 +81,9 @@ impl Decode<'_, ()> for Cip36KeyRegistration {
8181

8282
let mut cip36_key_registration = Cip36KeyRegistration::default();
8383

84+
// Record of founded keys. Check for duplicate keys in the map
85+
let mut found_keys: Vec<Cip36KeyRegistrationKeys> = Vec::new();
86+
8487
// Record of errors found during decoding
8588
let mut err_report = Vec::new();
8689

@@ -90,11 +93,11 @@ impl Decode<'_, ()> for Cip36KeyRegistration {
9093
if let Some(key) = Cip36KeyRegistrationKeys::from_repr(key) {
9194
match key {
9295
Cip36KeyRegistrationKeys::VotingKey => {
93-
if !cip36_key_registration.voting_pks.is_empty() {
96+
if found_keys.contains(&key) {
9497
err_report.push(format!(
9598
"Duplicate key in CIP36 Key Registration voting key at item {} in map", index + 1),
9699
);
97-
continue;
100+
continue;
98101
}
99102
if let Some((is_cip36, voting_keys)) = decode_voting_key(d, &mut err_report)
100103
{
@@ -103,7 +106,7 @@ impl Decode<'_, ()> for Cip36KeyRegistration {
103106
}
104107
},
105108
Cip36KeyRegistrationKeys::StakePk => {
106-
if cip36_key_registration.stake_pk.is_some() {
109+
if found_keys.contains(&key) {
107110
err_report.push(format!(
108111
"Duplicate key in CIP36 Key Registration stake public key at item {} in map", index + 1),
109112
);
@@ -114,56 +117,66 @@ impl Decode<'_, ()> for Cip36KeyRegistration {
114117
}
115118
},
116119
Cip36KeyRegistrationKeys::PaymentAddr => {
117-
if cip36_key_registration.payment_addr.is_some() {
120+
if found_keys.contains(&key) {
118121
err_report.push(format!(
119122
"Duplicate key in CIP36 Key Registration payment address at item {} in map", index + 1),
120123
);
121124
continue;
122125
}
123126
if let Some(shelley_addr) = decode_payment_addr(d, &mut err_report) {
124127
cip36_key_registration.payment_addr = Some(shelley_addr.clone());
125-
cip36_key_registration.is_payable = Some(!shelley_addr.payment().is_script());
128+
cip36_key_registration.is_payable =
129+
Some(!shelley_addr.payment().is_script());
126130
}
127131
},
128132
Cip36KeyRegistrationKeys::Nonce => {
129-
if cip36_key_registration.raw_nonce.is_some() {
133+
if found_keys.contains(&key) {
130134
err_report.push(format!(
131-
"Duplicate key in CIP36 Key Registration nonce at item {} in map", index + 1),
132-
);
135+
"Duplicate key in CIP36 Key Registration nonce at item {} in map",
136+
index + 1
137+
));
133138
continue;
134139
}
135140
if let Some(nonce) = decode_nonce(d, &mut err_report) {
136141
cip36_key_registration.raw_nonce = Some(nonce);
137142
}
138143
},
139144
Cip36KeyRegistrationKeys::Purpose => {
140-
if cip36_key_registration.purpose.is_some() {
145+
if found_keys.contains(&key) {
141146
err_report.push(format!(
142-
"Duplicate key in CIP36 Key Registration purpose at item {} in map", index + 1),
143-
);
147+
"Duplicate key in CIP36 Key Registration purpose at item {} in map",
148+
index + 1
149+
));
144150
continue;
145151
}
146152
if let Some(purpose) = decode_purpose(d, &mut err_report) {
147-
cip36_key_registration.purpose = Some(purpose);
153+
cip36_key_registration.purpose = purpose;
148154
}
149155
},
150156
}
157+
// Update the founded keys.
158+
found_keys.push(key);
151159
}
152160
}
153161

154-
if cip36_key_registration.voting_pks.is_empty() {
155-
err_report.push("Missing required key in CIP36 Key Registration: Voting Key".to_string());
162+
if !found_keys.contains(&Cip36KeyRegistrationKeys::VotingKey) {
163+
err_report
164+
.push("Missing required key in CIP36 Key Registration: Voting Key".to_string());
156165
}
157166

158-
if cip36_key_registration.stake_pk.is_none() {
159-
err_report.push("Missing required key in CIP36 Key Registration: Stake Public Key".to_string());
167+
if !found_keys.contains(&Cip36KeyRegistrationKeys::StakePk) {
168+
err_report.push(
169+
"Missing required key in CIP36 Key Registration: Stake Public Key".to_string(),
170+
);
160171
}
161172

162-
if cip36_key_registration.payment_addr.is_none() {
163-
err_report.push("Missing required key in CIP36 Key Registration: Payment Address".to_string());
173+
if !found_keys.contains(&Cip36KeyRegistrationKeys::PaymentAddr) {
174+
err_report.push(
175+
"Missing required key in CIP36 Key Registration: Payment Address".to_string(),
176+
);
164177
}
165178

166-
if cip36_key_registration.raw_nonce.is_none() {
179+
if !found_keys.contains(&Cip36KeyRegistrationKeys::Nonce) {
167180
err_report.push("Missing required key in CIP36 Key Registration: Nonce".to_string());
168181
}
169182

@@ -461,6 +474,6 @@ mod tests {
461474
let mut err_report = Vec::new();
462475
let nonce = decode_nonce(&mut decoder, &mut err_report);
463476
assert!(err_report.is_empty());
464-
assert_eq!(nonce.unwrap(), 21562833);
477+
assert_eq!(nonce.unwrap(), 21_562_833);
465478
}
466479
}

rust/cardano-blockchain-types/src/metadata/cip36/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ impl Cip36 {
151151

152152
/// Get the purpose from the registration.
153153
#[must_use]
154-
pub fn purpose(&self) -> Option<u64> {
154+
pub fn purpose(&self) -> u64 {
155155
self.key_registration.purpose
156156
}
157157

rust/cardano-blockchain-types/src/metadata/cip36/validation.rs

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,13 @@ pub(crate) fn validate_signature(
3232
if let Some(stake_pk) = cip36.stake_pk() {
3333
if let Ok(()) = stake_pk.verify_strict(hash.as_bytes(), &sig) {
3434
return true;
35-
} else {
36-
validation_report.push("Validate CIP36 Signature, cannot verify signature".to_string());
37-
return false;
3835
}
39-
} else {
40-
validation_report.push("Validate CIP36 Signature, stake public key is missing".to_string());
36+
validation_report.push("Validate CIP36 Signature, cannot verify signature".to_string());
4137
return false;
4238
}
39+
40+
validation_report.push("Validate CIP36 Signature, stake public key is missing".to_string());
41+
false
4342
}
4443

4544
/// Validate the payment address network against the given network.
@@ -78,19 +77,14 @@ pub(crate) fn validate_voting_keys(cip36: &Cip36, validation_report: &mut Vec<St
7877

7978
/// Validate the purpose.
8079
pub(crate) fn validate_purpose(cip36: &Cip36, validation_report: &mut Vec<String>) -> bool {
81-
if let Some(purpose) = cip36.purpose() {
82-
if cip36.is_strict_catalyst() && purpose != PROJECT_CATALYST_PURPOSE {
83-
validation_report.push(format!(
84-
"Validate CIP-36 Purpose, registration contains unknown purpose: {purpose}"
85-
));
86-
return false;
87-
}
88-
true
89-
} else {
90-
validation_report
91-
.push("Validate CIP-36 Purpose, registration purpose is missing".to_string());
92-
false
80+
if cip36.is_strict_catalyst() && cip36.purpose() != PROJECT_CATALYST_PURPOSE {
81+
validation_report.push(format!(
82+
"Validate CIP-36 Purpose, registration contains unknown purpose: {}",
83+
cip36.purpose()
84+
));
85+
return false;
9386
}
87+
true
9488
}
9589

9690
#[cfg(test)]
@@ -196,21 +190,20 @@ mod tests {
196190

197191
#[test]
198192
fn test_validate_purpose() {
199-
let mut cip36 = create_empty_cip36(true);
200-
cip36.key_registration.purpose = Some(0);
193+
let cip36 = create_empty_cip36(true);
201194
let mut report = Vec::new();
202195

203196
let valid = validate_purpose(&cip36, &mut report);
204197

205198
assert_eq!(report.len(), 0);
206-
assert_eq!(cip36.purpose(), Some(0));
199+
assert_eq!(cip36.purpose(), 0);
207200
assert!(valid);
208201
}
209202

210203
#[test]
211204
fn test_validate_invalid_purpose() {
212205
let mut cip36 = create_empty_cip36(true);
213-
cip36.key_registration.purpose = Some(1);
206+
cip36.key_registration.purpose = 1;
214207
let mut report = Vec::new();
215208

216209
let valid = validate_purpose(&cip36, &mut report);
@@ -220,7 +213,7 @@ mod tests {
220213
.first()
221214
.expect("Failed to get the first index")
222215
.contains("unknown purpose"));
223-
assert_eq!(cip36.purpose(), Some(1));
216+
assert_eq!(cip36.purpose(), 1);
224217
assert!(!valid);
225218
}
226219
}

0 commit comments

Comments
 (0)