Skip to content

Commit af71a30

Browse files
committed
fix: address XLS-70 credential review feedback
1 parent 3e2830b commit af71a30

12 files changed

Lines changed: 315 additions & 149 deletions

File tree

src/models/ledger/objects/deposit_preauth.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,13 @@ mod tests {
395395
),
396396
previous_txn_lgr_seq: 8,
397397
};
398-
assert!(deposit_preauth.get_errors().is_err());
398+
assert_eq!(
399+
deposit_preauth.get_errors().unwrap_err(),
400+
XRPLModelException::ValueTooLong {
401+
field: "authorize_credentials".into(),
402+
max: 8,
403+
found: 9,
404+
}
405+
);
399406
}
400407
}

src/models/requests/deposit_authorize.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ mod tests {
7777
"rDest11111111111111111111111111111".into(),
7878
"rSrc111111111111111111111111111111".into(),
7979
None,
80+
None,
8081
Some(LedgerIndex::Str("validated".into())),
8182
);
8283
let serialized = serde_json::to_string(&req).unwrap();

src/models/requests/ledger_entry.rs

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,33 @@ impl<'a> Request<'a> for LedgerEntry<'a> {
173173
}
174174
}
175175

176+
impl<'a> Default for LedgerEntry<'a> {
177+
fn default() -> Self {
178+
Self {
179+
common_fields: CommonFields {
180+
command: RequestMethod::LedgerEntry,
181+
id: None,
182+
},
183+
account_root: None,
184+
binary: None,
185+
check: None,
186+
credential: None,
187+
deposit_preauth: None,
188+
directory: None,
189+
escrow: None,
190+
index: None,
191+
ledger_lookup: Some(LookupByLedgerRequest {
192+
ledger_hash: None,
193+
ledger_index: None,
194+
}),
195+
offer: None,
196+
payment_channel: None,
197+
ripple_state: None,
198+
ticket: None,
199+
}
200+
}
201+
}
202+
176203
impl<'a> LedgerEntry<'a> {
177204
pub fn new(
178205
id: Option<Cow<'a, str>>,
@@ -231,26 +258,14 @@ mod test_ledger_entry_errors {
231258

232259
#[test]
233260
fn test_fields_error() {
234-
let ledger_entry = LedgerEntry::new(
235-
None,
236-
Some("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into()),
237-
None,
238-
None,
239-
None,
240-
None,
241-
None,
242-
None,
243-
None,
244-
None,
245-
None,
246-
Some(Offer {
261+
let ledger_entry = LedgerEntry {
262+
account_root: Some("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into()),
263+
offer: Some(Offer {
247264
account: "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into(),
248265
seq: 359,
249266
}),
250-
None,
251-
None,
252-
None,
253-
);
267+
..Default::default()
268+
};
254269
let _expected = XRPLModelException::ExpectedOneOf(&[
255270
"index",
256271
"account_root",
@@ -272,26 +287,14 @@ mod test_ledger_entry_errors {
272287

273288
#[test]
274289
fn test_serde() {
275-
let req = LedgerEntry::new(
276-
None,
277-
Some("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into()),
278-
None,
279-
None,
280-
None,
281-
None,
282-
None,
283-
None,
284-
None,
285-
None,
286-
None,
287-
Some(Offer {
290+
let req = LedgerEntry {
291+
account_root: Some("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into()),
292+
offer: Some(Offer {
288293
account: "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn".into(),
289294
seq: 359,
290295
}),
291-
None,
292-
None,
293-
None,
294-
);
296+
..Default::default()
297+
};
295298
let serialized = serde_json::to_string(&req).unwrap();
296299

297300
let deserialized: LedgerEntry = serde_json::from_str(&serialized).unwrap();

src/models/transactions/credential_accept.rs

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use crate::models::amount::XRPAmount;
77
use crate::models::transactions::CommonFields;
88
use crate::models::{
99
transactions::{Memo, Signer, Transaction, TransactionType},
10-
Model, XRPLModelException, XRPLModelResult,
10+
Model, XRPLModelResult,
1111
};
12-
use crate::models::{FlagCollection, NoFlags, ValidateCurrencies};
12+
use crate::models::{FlagCollection, NoFlags};
1313

1414
use super::CommonTransactionBuilder;
1515

@@ -18,16 +18,7 @@ use super::CommonTransactionBuilder;
1818
/// See CredentialAccept:
1919
/// `<https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0070-credentials>`
2020
#[skip_serializing_none]
21-
#[derive(
22-
Debug,
23-
Default,
24-
Serialize,
25-
Deserialize,
26-
PartialEq,
27-
Eq,
28-
Clone,
29-
xrpl_rust_macros::ValidateCurrencies,
30-
)]
21+
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)]
3122
#[serde(rename_all = "PascalCase")]
3223
pub struct CredentialAccept<'a> {
3324
/// The base fields for all transaction models.
@@ -41,8 +32,7 @@ pub struct CredentialAccept<'a> {
4132

4233
impl<'a> Model for CredentialAccept<'a> {
4334
fn get_errors(&self) -> XRPLModelResult<()> {
44-
self._get_credential_type_error()?;
45-
self.validate_currencies()
35+
self._get_credential_type_error()
4636
}
4737
}
4838

@@ -110,22 +100,7 @@ impl<'a> CredentialAccept<'a> {
110100

111101
impl<'a> CredentialAcceptError for CredentialAccept<'a> {
112102
fn _get_credential_type_error(&self) -> XRPLModelResult<()> {
113-
let len = self.credential_type.len();
114-
if len == 0 {
115-
Err(XRPLModelException::ValueTooShort {
116-
field: "credential_type".into(),
117-
min: 1,
118-
found: 0,
119-
})
120-
} else if len > 128 {
121-
Err(XRPLModelException::ValueTooLong {
122-
field: "credential_type".into(),
123-
max: 128,
124-
found: len,
125-
})
126-
} else {
127-
Ok(())
128-
}
103+
super::validate_credential_type(&self.credential_type)
129104
}
130105
}
131106

@@ -136,7 +111,7 @@ pub trait CredentialAcceptError {
136111
#[cfg(test)]
137112
mod tests {
138113
use super::*;
139-
use crate::models::Model;
114+
use crate::models::{Model, XRPLModelException};
140115
use alloc::borrow::Cow;
141116

142117
#[test]
@@ -166,47 +141,68 @@ mod tests {
166141
}
167142

168143
#[test]
169-
fn test_credential_type_length_validation() {
144+
fn test_credential_type_empty_error() {
170145
let tx = CredentialAccept {
171146
common_fields: CommonFields {
172147
account: "rSubject11111111111111111111111111".into(),
173148
transaction_type: TransactionType::CredentialAccept,
174149
..Default::default()
175150
},
176151
issuer: "rIssuer111111111111111111111111111".into(),
177-
credential_type: "".into(),
152+
credential_type: Cow::from(""),
178153
};
179-
assert!(tx.get_errors().is_err());
154+
assert_eq!(
155+
tx.get_errors().unwrap_err(),
156+
XRPLModelException::ValueTooShort {
157+
field: "credential_type".into(),
158+
min: 1,
159+
found: 0,
160+
}
161+
);
180162
}
181163

182164
#[test]
183-
fn test_credential_type_empty_error() {
165+
fn test_credential_type_too_long_error() {
166+
// 129 hex chars exceeds the 128 limit
167+
let too_long: Cow<'_, str> = Cow::from("A".repeat(129));
184168
let tx = CredentialAccept {
185169
common_fields: CommonFields {
186170
account: "rSubject11111111111111111111111111".into(),
187171
transaction_type: TransactionType::CredentialAccept,
188172
..Default::default()
189173
},
190174
issuer: "rIssuer111111111111111111111111111".into(),
191-
credential_type: Cow::from(""),
175+
credential_type: too_long,
192176
};
193-
assert!(tx.get_errors().is_err());
177+
assert_eq!(
178+
tx.get_errors().unwrap_err(),
179+
XRPLModelException::ValueTooLong {
180+
field: "credential_type".into(),
181+
max: 128,
182+
found: 129,
183+
}
184+
);
194185
}
195186

196187
#[test]
197-
fn test_credential_type_too_long_error() {
198-
// 129 hex chars exceeds the 128 limit
199-
let too_long: Cow<'_, str> = Cow::from("A".repeat(129));
188+
fn test_credential_type_non_hex_error() {
200189
let tx = CredentialAccept {
201190
common_fields: CommonFields {
202191
account: "rSubject11111111111111111111111111".into(),
203192
transaction_type: TransactionType::CredentialAccept,
204193
..Default::default()
205194
},
206195
issuer: "rIssuer111111111111111111111111111".into(),
207-
credential_type: too_long,
196+
credential_type: "NOT_HEX".into(),
208197
};
209-
assert!(tx.get_errors().is_err());
198+
assert_eq!(
199+
tx.get_errors().unwrap_err(),
200+
XRPLModelException::InvalidValueFormat {
201+
field: "credential_type".into(),
202+
format: "hexadecimal".into(),
203+
found: "NOT_HEX".into(),
204+
}
205+
);
210206
}
211207

212208
#[test]

0 commit comments

Comments
 (0)