Repository navigation
feat: Support LendingProtocolV1_1
#373
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Seeds for failure cases proptest has generated in the past. It is | ||
| # automatically read and these particular cases re-run before any | ||
| # novel cases are generated. | ||
| # | ||
| # It is recommended to check this file in to source control so that | ||
| # everyone who runs the test benefits from these saved cases. | ||
| cc 98d6525f48ecdceefc3011f86b79524f2cff7fdacb0f866cde56124f1724e5c4 # shrinks to count = 1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,8 +17,14 @@ use crate::{ | |||||
| }, | ||||||
| core::binarycodec::encode, | ||||||
| models::{ | ||||||
| requests::{ | ||||||
| account_info::AccountInfo, ledger_entry::LedgerEntry, server_state::ServerState, | ||||||
| submit::Submit, | ||||||
| }, | ||||||
| results::{ | ||||||
| ledger_entry::LedgerEntry as LedgerEntryResult, | ||||||
| server_state::ServerState as ServerStateResult, submit::Submit as SubmitResult, | ||||||
| }, | ||||||
| transactions::{Transaction, TransactionType}, | ||||||
| Model, XRPAmount, XRPLModelException, | ||||||
| }, | ||||||
|
|
@@ -194,6 +200,18 @@ where | |||||
| }; | ||||||
| } | ||||||
| let mut base_fee_decimal: BigDecimal = base_fee.try_into()?; | ||||||
| // A `LoanSet` also pays one reference base fee per counterparty signer | ||||||
| // (rippled `LoanSet::calculateBaseFee`: `normalCost + signerCount * base`). | ||||||
| // `Fee` is a signing field, so it cannot be raised after the first party | ||||||
| // signs — it has to be right here, before the transaction is distributed | ||||||
| // for counterparty signing. | ||||||
| if transaction.get_transaction_type() == &TransactionType::LoanSet { | ||||||
| if let Some(client) = client { | ||||||
|
Collaborator
There was a problem hiding this comment. With no client, nothing is added for the counterparty, so the public offline |
||||||
| let counterparty_signers = get_counterparty_signers_count(transaction, client).await?; | ||||||
| let net_fee_decimal: BigDecimal = net_fee.clone().try_into()?; | ||||||
| base_fee_decimal += &(net_fee_decimal * BigDecimal::from(counterparty_signers)); | ||||||
| } | ||||||
| } | ||||||
| if let Some(signers_count) = signers_count { | ||||||
| // rippled charges one extra reference base fee per signer on top of the | ||||||
| // transaction's own base cost (Transactor::calculateBaseFee: | ||||||
|
|
@@ -210,6 +228,109 @@ where | |||||
| .into()) | ||||||
| } | ||||||
|
|
||||||
| /// The number of counterparty signatures a `LoanSet` will carry, for fee | ||||||
| /// purposes. | ||||||
| /// | ||||||
| /// The counterparty is the `Counterparty` account when the transaction names | ||||||
| /// one; otherwise the loan broker initiated it and the counterparty is the | ||||||
|
Collaborator
There was a problem hiding this comment. The roles are reversed here. When
Suggested change
|
||||||
| /// broker's owner, which is read from the `LoanBroker` ledger entry. An account | ||||||
| /// with a signer list contributes one signature per signer entry; an account | ||||||
| /// without one signs once. | ||||||
| /// | ||||||
| /// This is an upper bound, not the exact count. rippled charges on the | ||||||
| /// signatures the transaction actually carries | ||||||
| /// (`max(1, |CounterpartySignature.Signers|)`), but those do not exist yet at | ||||||
| /// autofill time — the counterparty signs later, possibly on another machine — | ||||||
| /// so the signer list is the only thing available to size the fee from. A | ||||||
| /// counterparty that owns a signer list but single-signs therefore overpays by | ||||||
| /// one base fee per unused signer entry; it never underpays, which would cost a | ||||||
| /// `telINSUF_FEE_P` and a re-signing round trip, since `Fee` is a signing field. | ||||||
| /// Callers who know the exact count can set `Fee` themselves before calling | ||||||
| /// `autofill`, which skips this calculation entirely. | ||||||
| /// | ||||||
| /// Mirrors `fetchCounterPartySignersCount` in xrpl.js's `autofill`, which sizes | ||||||
| /// the fee the same way. | ||||||
| async fn get_counterparty_signers_count<'a, T, F, C>( | ||||||
| transaction: &T, | ||||||
| client: &C, | ||||||
| ) -> XRPLHelperResult<u32> | ||||||
| where | ||||||
| T: Transaction<'a, F>, | ||||||
| F: IntoEnumIterator + Serialize + Debug + PartialEq, | ||||||
| C: XRPLAsyncClient, | ||||||
| { | ||||||
| fn unquote(value: String) -> String { | ||||||
| value.trim_matches('"').to_string() | ||||||
| } | ||||||
|
|
||||||
| let counterparty = match transaction.get_field_value("Counterparty")? { | ||||||
| Some(counterparty) => unquote(counterparty), | ||||||
| None => { | ||||||
| let loan_broker_id = transaction | ||||||
| .get_field_value("LoanBrokerID")? | ||||||
| .map(unquote) | ||||||
| .ok_or_else(|| { | ||||||
| XRPLModelException::MissingField( | ||||||
| "LoanBrokerID (required to resolve the LoanSet counterparty)".to_string(), | ||||||
| ) | ||||||
| })?; | ||||||
| let response = client | ||||||
| .request( | ||||||
| LedgerEntry { | ||||||
| index: Some(loan_broker_id.into()), | ||||||
| ..Default::default() | ||||||
| } | ||||||
| .into(), | ||||||
| ) | ||||||
| .await?; | ||||||
| let result: LedgerEntryResult = response.try_into()?; | ||||||
| let node = result.node.ok_or_else(|| { | ||||||
| XRPLModelException::MissingField("LoanBroker ledger entry".to_string()) | ||||||
| })?; | ||||||
| node["Owner"] | ||||||
| .as_str() | ||||||
| .ok_or_else(|| XRPLModelException::MissingField("LoanBroker.Owner".to_string()))? | ||||||
| .to_string() | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| let response = client | ||||||
| .request( | ||||||
| AccountInfo::new( | ||||||
| None, | ||||||
| counterparty.into(), | ||||||
| None, | ||||||
| None, | ||||||
| None, | ||||||
| None, | ||||||
| Some(true), | ||||||
| ) | ||||||
| .into(), | ||||||
| ) | ||||||
| .await?; | ||||||
| let result = response | ||||||
| .raw_result | ||||||
| .ok_or_else(|| XRPLModelException::MissingField("account_info result".to_string()))?; | ||||||
|
|
||||||
| // `signer_lists` sits at the root of the response under api_version 2 but | ||||||
| // nested inside `account_data` under api_version 1, which is what rippled | ||||||
| // serves when the request does not ask for a version — so accept either. | ||||||
| let signer_lists = result.get("signer_lists").or_else(|| { | ||||||
| result | ||||||
| .get("account_data") | ||||||
| .and_then(|d| d.get("signer_lists")) | ||||||
| }); | ||||||
|
|
||||||
| // An account without a signer list signs once; otherwise it contributes one | ||||||
| // signature per signer entry. | ||||||
| Ok(signer_lists | ||||||
| .and_then(|lists| lists.get(0)) | ||||||
| .and_then(|list| list.get("SignerEntries")) | ||||||
| .and_then(|entries| entries.as_array()) | ||||||
| .map(|entries| entries.len() as u32) | ||||||
| .unwrap_or(1)) | ||||||
| } | ||||||
|
|
||||||
| async fn get_owner_reserve_from_response( | ||||||
| client: &impl XRPLAsyncClient, | ||||||
| ) -> XRPLHelperResult<XRPAmount<'_>> { | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -691,6 +691,14 @@ impl Iterator for BinaryParser { | |
|
|
||
| pub(crate) const TRANSACTION_SIGNATURE_PREFIX: i32 = 0x53545800; | ||
| pub(crate) const TRANSACTION_MULTISIG_PREFIX: [u8; 4] = (0x534D5400u32).to_be_bytes(); | ||
| /// Inner transaction to sign as the counterparty — "CPT\0" (`fixCleanup3_4_0`). | ||
| /// | ||
| /// Each signature role covers a distinct prefix so a signature cannot be | ||
| /// replayed in another role (see rippled `signingPrefix`). | ||
| pub(crate) const COUNTERPARTY_TRANSACTION_SIGNATURE_PREFIX: [u8; 4] = (0x43505400u32).to_be_bytes(); | ||
| /// Inner transaction to multi-sign as the counterparty — "CPM\0" | ||
| /// (`fixCleanup3_4_0`). | ||
| pub(crate) const COUNTERPARTY_TRANSACTION_MULTISIG_PREFIX: [u8; 4] = (0x43504D00u32).to_be_bytes(); | ||
| pub(crate) const PAYMENT_CHANNEL_CLAIM_PREFIX: [u8; 4] = (0x434C4D00u32).to_be_bytes(); | ||
| pub(crate) const BATCH_PREFIX: [u8; 4] = (0x42434800u32).to_be_bytes(); | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The
fixCleanup3_4_0limitation is stated backwards.sign_loan_set_by_counterpartyalways signs withCPT/CPM(src/signing/mod.rslines 413 and 433), but rippled keeps expectingSTX/SMTfor counterparty signatures untilfixCleanup3_4_0is enabled (signingPrefixinsrc/libxrpl/protocol/Sign.cpp). So on a network that enables LendingProtocol first, every LoanSet signed by this SDK is rejected. Today Mainnet and Testnet have neither amendment enabled. This matches xrpl.js and xrpl-py, so I'm not asking for a legacy-prefix option, but users need to be told.The current sentence only applies to builds of unreleased
main(Lending isn't in 1.3.0), and it leaves out the direction that matters: signatures from this version are only valid wherefixCleanup3_4_0is enabled. The same note belongs in thesign_loan_set_by_counterpartydocs (lines 145-159 ofsrc/signing/mod.rs).Line 18 has a related mix-up:
CredentialIDsonVaultWithdrawandLoanBrokerCoverWithdrawisn't gated onLendingProtocolV1_1; rippled requiresCredentialsplusfixCleanup3_4_0(VaultWithdraw::checkExtraFeatures,LoanBrokerCoverWithdraw::checkExtraFeatures). Suggested wording for both: