From dea8982f3a2adb01d641adaf7e2b4d65400cf211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 13:05:50 +0000 Subject: [PATCH 1/7] test(ICP_Rosetta): pin down random-memo behavior in construction_payloads Add a unit test that reconstructs the same transfer three times without a caller-specified memo and asserts that at least two of the resulting memos differ. This captures the current behavior, where construction_payloads substitutes a fresh random memo whenever the caller omits one (DEFI-1800). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/request_handler/construction_parse.rs | 165 +++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs index 0e8be5d6438c..342d15adbd47 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs @@ -672,7 +672,7 @@ mod tests { models::{ Amount, ConstructionCombineRequest, ConstructionDeriveRequest, ConstructionParseRequest, ConstructionPayloadsRequest, - ConstructionPayloadsRequestMetadata, Currency, CurveType, Operation, + ConstructionPayloadsRequestMetadata, Currency, CurveType, NetworkIdentifier, Operation, OperationIdentifier, PublicKey, Signature, SignatureType, operation::OperationType, }, request_handler::RosettaRequestHandler, @@ -924,4 +924,167 @@ mod tests { check_metadata(metadata, parsed.metadata.unwrap()).unwrap() }); } + + /// Builds a `RosettaRequestHandler` together with a single ICP transfer + /// (debit + credit + fee) and the signer's public key, shared by the memo + /// tests below. + fn setup_transfer_test() -> ( + RosettaRequestHandler, + NetworkIdentifier, + Vec, + PublicKey, + ) { + let key = ic_ed25519::PrivateKey::generate_using_rng(&mut OsRng); + let ledger_client = futures::executor::block_on(LedgerClient::new( + Url::from_str("http://localhost:1234").unwrap(), + CanisterId::from_u64(1), + "TKN".into(), + CanisterId::from_u64(2), + None, + None, + true, + None, + false, + false, // optimize_search_indexes: disabled for tests + )) + .unwrap(); + let mock_canister_id_hex = "00000000000000000101"; + let initial_sync_complete = AtomicBool::new(true); + let handler = RosettaRequestHandler::new( + "Internet Computer".into(), + ledger_client.into(), + RosettaMetrics::new("TKN".into(), mock_canister_id_hex.into()), + Arc::new(initial_sync_complete), + ); + + let network_identifier = handler.network_id(); + let currency = Currency { + symbol: "TKN".into(), + decimals: 8, + metadata: None, + }; + + let pub_key = PublicKey { + hex_bytes: hex::encode(key.public_key().serialize_raw()), + curve_type: CurveType::Edwards25519, + }; + let account = handler + .construction_derive(ConstructionDeriveRequest { + network_identifier: network_identifier.clone(), + public_key: pub_key.clone(), + metadata: None, + }) + .unwrap() + .account_identifier; + + let operations = vec![ + Operation { + operation_identifier: OperationIdentifier { + index: 0, + network_index: None, + }, + related_operations: None, + type_: OperationType::Transaction.to_string(), + status: None, + account: account.clone(), + amount: Some(Amount { + value: "-100000000".into(), + currency: currency.clone(), + metadata: None, + }), + coin_change: None, + metadata: None, + }, + Operation { + operation_identifier: OperationIdentifier { + index: 1, + network_index: None, + }, + related_operations: None, + type_: OperationType::Transaction.to_string(), + status: None, + account: account.clone(), + amount: Some(Amount { + value: "100000000".into(), + currency: currency.clone(), + metadata: None, + }), + coin_change: None, + metadata: None, + }, + Operation { + operation_identifier: OperationIdentifier { + index: 2, + network_index: None, + }, + related_operations: None, + type_: OperationType::Fee.to_string(), + status: None, + account, + amount: Some(Amount { + value: "-1000000".into(), + currency, + metadata: None, + }), + coin_change: None, + metadata: None, + }, + ]; + + (handler, network_identifier, operations, pub_key) + } + + /// Returns the `memo` that `construction_payloads` embeds in the + /// transaction when invoked without a caller-specified memo. + fn memo_without_caller_memo( + handler: &RosettaRequestHandler, + network_identifier: &NetworkIdentifier, + operations: &[Operation], + pub_key: &PublicKey, + ) -> serde_json::Value { + let payloads = handler + .construction_payloads(ConstructionPayloadsRequest { + network_identifier: network_identifier.clone(), + operations: operations.to_vec(), + metadata: None, + public_keys: Some(vec![pub_key.clone()]), + }) + .unwrap(); + let parsed = handler + .construction_parse(ConstructionParseRequest { + network_identifier: network_identifier.clone(), + signed: false, + transaction: payloads.unsigned_transaction, + }) + .unwrap(); + parsed + .metadata + .expect("metadata should always be returned") + .get("memo") + .expect("memo should always be present") + .clone() + } + + // When the caller does not specify a memo, `construction_payloads` + // substitutes a fresh random memo on every invocation. Because the memo is + // part of the transaction hash, reconstructing the same transfer yields a + // different hash each time, which defeats the ledger's `created_at_time` + // based deduplication. This test pins down that behavior: three subsequent + // reconstructions of the same transfer produce at least two different + // memos. + #[test] + fn test_payloads_without_memo_produces_varying_memos() { + let (handler, network_identifier, operations, pub_key) = setup_transfer_test(); + + let memos: Vec = (0..3) + .map(|_| memo_without_caller_memo(&handler, &network_identifier, &operations, &pub_key)) + .collect(); + + let distinct: std::collections::HashSet = + memos.iter().map(|m| m.to_string()).collect(); + assert!( + distinct.len() >= 2, + "expected at least two different memos across three reconstructions, got {memos:?}" + ); + } } From 085c2447ead86eae7ef8a73cbae50b48d8781a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 13:08:39 +0000 Subject: [PATCH 2/7] fix(ICP_Rosetta): use a deterministic memo of 0 instead of a random one When the caller omits the memo in construction_payloads, substitute Memo(0) (the ledger's "no memo" value) instead of a random memo. The random memo was made part of the transaction hash, so reconstructing the same transfer produced a different hash on every call and defeated the ledger's created_at_time based deduplication, allowing a retried transfer to be applied twice. Using a deterministic memo lets the ledger dedup catch such retries. This also drops the now-unused rand import and removes the stale FIXME. Note: the randomness test added in the previous commit now fails on purpose; it is updated to assert the new behavior in the following commit. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../icp/src/request_handler/construction_payloads.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs b/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs index a160a6fe0634..04bf8f8296ab 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs @@ -29,7 +29,6 @@ use ic_types::{ messages::{Blob, HttpCanisterUpdate, MessageId}, }; use icp_ledger::{Memo, Operation, SendArgs, Tokens}; -use rand::Rng; use rosetta_core::convert::principal_id_from_public_key; use std::{collections::HashMap, sync::Arc, time::Duration}; use tracing::log::debug; @@ -89,12 +88,16 @@ impl RosettaRequestHandler { .map(ic_ledger_core::timestamp::TimeStamp::from_nanos_since_unix_epoch) .unwrap_or_else(|| std::time::SystemTime::now().into()); - // FIXME: the memo field needs to be associated with the operation + // When the caller does not specify a memo, default to Memo(0) (the + // ledger's "no memo" value) rather than a random memo. A random memo + // would change the transaction hash on every reconstruction of the + // same transfer, defeating the ledger's created_at_time based + // deduplication and allowing a retried transfer to be applied twice. let memo: Memo = meta .as_ref() .and_then(|meta| meta.memo) .map(Memo) - .unwrap_or_else(|| Memo(rand::thread_rng().r#gen())); + .unwrap_or(Memo(0)); let mut ingress_expiries = vec![]; let mut now = ingress_start; From 7f95b1a0dc6636543959cacbebc0af698ef30b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 13:10:41 +0000 Subject: [PATCH 3/7] test(ICP_Rosetta): assert deterministic zero memo on reconstruction Update the memo test to match the new behavior: three subsequent reconstructions of the same transfer without a caller-specified memo must all produce a memo of 0, so that the ledger's created_at_time based deduplication catches a retried transfer. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/request_handler/construction_parse.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs index 342d15adbd47..271f201d5211 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs @@ -1065,26 +1065,24 @@ mod tests { .clone() } - // When the caller does not specify a memo, `construction_payloads` - // substitutes a fresh random memo on every invocation. Because the memo is - // part of the transaction hash, reconstructing the same transfer yields a - // different hash each time, which defeats the ledger's `created_at_time` - // based deduplication. This test pins down that behavior: three subsequent - // reconstructions of the same transfer produce at least two different - // memos. + // When the caller does not specify a memo, `construction_payloads` uses a + // deterministic memo of 0 (the ledger's "no memo" value). Because the memo + // is part of the transaction hash, reconstructing the same transfer must + // therefore yield the same hash every time, so that the ledger's + // `created_at_time` based deduplication can catch a retried transfer. This + // test verifies that three subsequent reconstructions of the same transfer + // all produce a memo of 0. #[test] - fn test_payloads_without_memo_produces_varying_memos() { + fn test_payloads_without_memo_uses_deterministic_zero_memo() { let (handler, network_identifier, operations, pub_key) = setup_transfer_test(); let memos: Vec = (0..3) .map(|_| memo_without_caller_memo(&handler, &network_identifier, &operations, &pub_key)) .collect(); - let distinct: std::collections::HashSet = - memos.iter().map(|m| m.to_string()).collect(); assert!( - distinct.len() >= 2, - "expected at least two different memos across three reconstructions, got {memos:?}" + memos.iter().all(|memo| *memo == serde_json::json!(0)), + "expected a deterministic memo of 0 on every reconstruction, got {memos:?}" ); } } From d1a3a525d9bc9a34b8d0cd0ec2f83e7e4191e738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 13:13:58 +0000 Subject: [PATCH 4/7] docs(ICP_Rosetta): document deterministic zero memo and update CHANGELOG Update the `memo` field doc-comment (no longer "a random number by default") and the stale test comment, and add a CHANGELOG entry for the change from a random to a deterministic memo of 0 in construction_payloads. Co-Authored-By: Claude Opus 4.8 (1M context) --- rs/rosetta-api/icp/CHANGELOG.md | 2 ++ rs/rosetta-api/icp/src/models.rs | 2 +- rs/rosetta-api/icp/src/request_handler/construction_parse.rs | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/rs/rosetta-api/icp/CHANGELOG.md b/rs/rosetta-api/icp/CHANGELOG.md index bd4c81bfbdd4..915a356d1ee9 100644 --- a/rs/rosetta-api/icp/CHANGELOG.md +++ b/rs/rosetta-api/icp/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Changed +- When no memo is specified for a transfer in `construction/payloads`, use a deterministic memo of 0 instead of a random one. Reconstructing the same transfer now produces the same transaction hash, so the ledger's `created_at_time` based deduplication catches retried transfers instead of applying them twice. ([#10537](https://github.com/dfinity/ic/pull/10537)) ## [2.1.9] - 2026-02-02 ### Added diff --git a/rs/rosetta-api/icp/src/models.rs b/rs/rosetta-api/icp/src/models.rs index ed80e63ea44d..5e980b322ea6 100644 --- a/rs/rosetta-api/icp/src/models.rs +++ b/rs/rosetta-api/icp/src/models.rs @@ -200,7 +200,7 @@ impl TryFrom for ParsedTransaction { #[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] pub struct ConstructionPayloadsRequestMetadata { /// The memo to use for a ledger transfer. - /// A random number is used by default. + /// Defaults to 0 if not specified. #[serde(skip_serializing_if = "Option::is_none")] pub memo: Option, diff --git a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs index 271f201d5211..36c21472b658 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs @@ -811,7 +811,8 @@ mod tests { ) -> std::result::Result<(), TestCaseError> { match expected_metadata { None => { - // memo and created_at_time should always be set but the value is random + // memo and created_at_time are always populated by construction_payloads + // (memo defaults to 0, created_at_time to the current time) prop_assert!( actual_metadata.contains_key("memo"), "Metadata should always contain a memo" From 8857eb9c5543b8233d3bb9cf901ef9ded4642bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 14:32:21 +0000 Subject: [PATCH 5/7] chore(ICP_Rosetta): drop now-unused rand dependency The random memo was the only user of the rand crate in ic-rosetta-api; remove it from Cargo.toml and the BUILD.bazel targets. rand_chacha (used by tests) is retained. Co-Authored-By: Claude Opus 4.8 (1M context) --- rs/rosetta-api/icp/BUILD.bazel | 6 ------ rs/rosetta-api/icp/Cargo.toml | 1 - 2 files changed, 7 deletions(-) diff --git a/rs/rosetta-api/icp/BUILD.bazel b/rs/rosetta-api/icp/BUILD.bazel index 87c56e459c03..3a59d79d33ed 100644 --- a/rs/rosetta-api/icp/BUILD.bazel +++ b/rs/rosetta-api/icp/BUILD.bazel @@ -53,7 +53,6 @@ rust_library( "@crate_index//:ic-management-canister-types", "@crate_index//:lazy_static", "@crate_index//:num-bigint", - "@crate_index//:rand", "@crate_index//:reqwest", "@crate_index//:rusqlite", "@crate_index//:serde", @@ -108,7 +107,6 @@ rust_binary( "@crate_index//:ic-management-canister-types", "@crate_index//:lazy_static", "@crate_index//:num-bigint", - "@crate_index//:rand", "@crate_index//:reqwest", "@crate_index//:rolling-file", "@crate_index//:rusqlite", @@ -167,7 +165,6 @@ rust_binary( "@crate_index//:ic-management-canister-types", "@crate_index//:lazy_static", "@crate_index//:num-bigint", - "@crate_index//:rand", "@crate_index//:reqwest", "@crate_index//:rolling-file", "@crate_index//:rusqlite", @@ -244,7 +241,6 @@ rust_test( "@crate_index//:num-traits", "@crate_index//:proptest", "@crate_index//:prost", - "@crate_index//:rand", "@crate_index//:rand_chacha", "@crate_index//:reqwest", "@crate_index//:rolling-file", @@ -338,7 +334,6 @@ rust_test_suite_with_extra_srcs( "@crate_index//:num-traits", "@crate_index//:proptest", "@crate_index//:prost", - "@crate_index//:rand", "@crate_index//:rand_chacha", "@crate_index//:reqwest", "@crate_index//:rolling-file", @@ -451,7 +446,6 @@ rust_test( "@crate_index//:num-traits", "@crate_index//:proptest", "@crate_index//:prost", - "@crate_index//:rand", "@crate_index//:rand_chacha", "@crate_index//:reqwest", "@crate_index//:rolling-file", diff --git a/rs/rosetta-api/icp/Cargo.toml b/rs/rosetta-api/icp/Cargo.toml index 132a9213e93d..b2382dfcd42c 100644 --- a/rs/rosetta-api/icp/Cargo.toml +++ b/rs/rosetta-api/icp/Cargo.toml @@ -36,7 +36,6 @@ icrc-ledger-types = { path = "../../../packages/icrc-ledger-types" } lazy_static = { workspace = true } num-bigint = { workspace = true } on_wire = { path = "../../rust_canisters/on_wire" } -rand = { workspace = true } registry-canister = { path = "../../registry/canister" } reqwest = { workspace = true } rolling-file = { workspace = true } From 616f5795277cd9f92b88ceb1420b8ba4f54d5c0e Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Mon, 22 Jun 2026 14:37:03 +0000 Subject: [PATCH 6/7] Automatically updated Cargo*.lock --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c2e1fc00702c..6d933143d57a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13834,7 +13834,6 @@ dependencies = [ "pocket-ic", "proptest", "prost 0.13.5", - "rand 0.8.5", "rand_chacha 0.3.1", "registry-canister", "reqwest", From a0655be32bd60883eae190e1888e7e76b25d9dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 22 Jun 2026 15:00:42 +0000 Subject: [PATCH 7/7] test(ICP_Rosetta): address review comments on the memo determinism test - Strengthen the test to pin all time-based inputs and assert that three reconstructions of the same transfer yield byte-identical unsigned transactions (hence the same transaction hash), in addition to a memo of 0. The comment now matches what is actually verified. - Extract the shared handler/key/operations setup into setup_transfer_test and use it from test_payloads_parse_identity too, removing the duplicated boilerplate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/request_handler/construction_parse.rs | 199 +++++------------- 1 file changed, 57 insertions(+), 142 deletions(-) diff --git a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs index 36c21472b658..ad2e33a02918 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs @@ -681,106 +681,8 @@ mod tests { #[test] fn test_payloads_parse_identity() { - let key = ic_ed25519::PrivateKey::generate_using_rng(&mut OsRng); - let ledger_client = futures::executor::block_on(LedgerClient::new( - Url::from_str("http://localhost:1234").unwrap(), - CanisterId::from_u64(1), - "TKN".into(), - CanisterId::from_u64(2), - None, - None, - true, - None, - false, - false, // optimize_search_indexes: disabled for tests - )) - .unwrap(); - // Create a mock canister ID for testing - let mock_canister_id_hex = "00000000000000000101"; - let initial_sync_complete = AtomicBool::new(true); - let handler = RosettaRequestHandler::new( - "Internet Computer".into(), - ledger_client.into(), - RosettaMetrics::new("TKN".into(), mock_canister_id_hex.into()), - Arc::new(initial_sync_complete), - ); + let (handler, network_identifier, operations, pub_key, key) = setup_transfer_test(); - // get the nextwork identifier - let network_identifier = handler.network_id(); - let currency = Currency { - symbol: "TKN".into(), - decimals: 8, - metadata: None, - }; - - // get the account from the public key - let pub_key = crate::models::PublicKey { - hex_bytes: hex::encode(key.public_key().serialize_raw()), - curve_type: CurveType::Edwards25519, - }; - let account = handler - .construction_derive(ConstructionDeriveRequest { - network_identifier: network_identifier.clone(), - public_key: pub_key.clone(), - metadata: None, - }) - .unwrap() - .account_identifier; - - // create the unsigned transaction - let operations = vec![ - Operation { - operation_identifier: OperationIdentifier { - index: 0, - network_index: None, - }, - related_operations: None, - type_: OperationType::Transaction.to_string(), - status: None, - account: account.clone(), - amount: Some(Amount { - value: "-100000000".into(), - currency: currency.clone(), - metadata: None, - }), - coin_change: None, - metadata: None, - }, - Operation { - operation_identifier: OperationIdentifier { - index: 1, - network_index: None, - }, - related_operations: None, - type_: OperationType::Transaction.to_string(), - status: None, - account: account.clone(), - amount: Some(Amount { - value: "100000000".into(), - currency: currency.clone(), - metadata: None, - }), - coin_change: None, - metadata: None, - }, - Operation { - operation_identifier: OperationIdentifier { - index: 2, - network_index: None, - }, - related_operations: None, - type_: OperationType::Fee.to_string(), - status: None, - account, - amount: Some(Amount { - value: "-1000000".into(), - currency, - metadata: None, - }), - coin_change: None, - metadata: None, - }, - ]; let gen_opt_u64 = proptest::option::of(proptest::prelude::any::()); const ONE_HOUR_NANOS: u64 = 60 * 60 * 1_000_000_000; let now = SystemTime::now() @@ -927,13 +829,14 @@ mod tests { } /// Builds a `RosettaRequestHandler` together with a single ICP transfer - /// (debit + credit + fee) and the signer's public key, shared by the memo - /// tests below. + /// (debit + credit + fee) and the signer's public and private keys, shared + /// by the construction tests below. fn setup_transfer_test() -> ( RosettaRequestHandler, NetworkIdentifier, Vec, PublicKey, + ic_ed25519::PrivateKey, ) { let key = ic_ed25519::PrivateKey::generate_using_rng(&mut OsRng); let ledger_client = futures::executor::block_on(LedgerClient::new( @@ -1032,58 +935,70 @@ mod tests { }, ]; - (handler, network_identifier, operations, pub_key) + (handler, network_identifier, operations, pub_key, key) } - /// Returns the `memo` that `construction_payloads` embeds in the - /// transaction when invoked without a caller-specified memo. - fn memo_without_caller_memo( - handler: &RosettaRequestHandler, - network_identifier: &NetworkIdentifier, - operations: &[Operation], - pub_key: &PublicKey, - ) -> serde_json::Value { - let payloads = handler - .construction_payloads(ConstructionPayloadsRequest { - network_identifier: network_identifier.clone(), - operations: operations.to_vec(), - metadata: None, - public_keys: Some(vec![pub_key.clone()]), - }) - .unwrap(); + // When the caller does not specify a memo, `construction_payloads` uses a + // deterministic memo of 0 (the value the ledger treats as "no memo") + // instead of a random one. With the time-based inputs pinned, reconstructing + // the same transfer must therefore produce a byte-identical unsigned + // transaction every time -- and hence the same transaction hash -- so that + // the ledger's `created_at_time` based deduplication can catch a retried + // transfer. This test reconstructs the same transfer three times and checks + // both that the unsigned transactions are identical and that the embedded + // memo is 0. + #[test] + fn test_payloads_without_memo_is_deterministic() { + let (handler, network_identifier, operations, pub_key, _key) = setup_transfer_test(); + + // Pin all time-based inputs so that the only thing that could vary + // across reconstructions is the (previously random) memo. + const NANOS: u64 = 1_000_000_000; + let created_at_time = 1_700_000_000 * NANOS; + let metadata = ConstructionPayloadsRequestMetadata { + memo: None, + created_at_time: Some(created_at_time), + ingress_start: Some(created_at_time), + ingress_end: Some(created_at_time + 600 * NANOS), + }; + + let reconstruct = || { + handler + .construction_payloads(ConstructionPayloadsRequest { + network_identifier: network_identifier.clone(), + operations: operations.clone(), + metadata: Some(metadata.clone().try_into().unwrap()), + public_keys: Some(vec![pub_key.clone()]), + }) + .unwrap() + .unsigned_transaction + }; + + let unsigned_transactions: Vec = (0..3).map(|_| reconstruct()).collect(); + + // Reconstruction must be deterministic: identical bytes => identical + // transaction hash, which is what lets the ledger deduplicate retries. + assert!( + unsigned_transactions + .iter() + .all(|tx| *tx == unsigned_transactions[0]), + "expected identical unsigned transactions across reconstructions, got {unsigned_transactions:?}" + ); + + // And the memo embedded in the (memo-less) transfer must be 0. let parsed = handler .construction_parse(ConstructionParseRequest { network_identifier: network_identifier.clone(), signed: false, - transaction: payloads.unsigned_transaction, + transaction: unsigned_transactions[0].clone(), }) .unwrap(); - parsed + let memo = parsed .metadata .expect("metadata should always be returned") .get("memo") .expect("memo should always be present") - .clone() - } - - // When the caller does not specify a memo, `construction_payloads` uses a - // deterministic memo of 0 (the ledger's "no memo" value). Because the memo - // is part of the transaction hash, reconstructing the same transfer must - // therefore yield the same hash every time, so that the ledger's - // `created_at_time` based deduplication can catch a retried transfer. This - // test verifies that three subsequent reconstructions of the same transfer - // all produce a memo of 0. - #[test] - fn test_payloads_without_memo_uses_deterministic_zero_memo() { - let (handler, network_identifier, operations, pub_key) = setup_transfer_test(); - - let memos: Vec = (0..3) - .map(|_| memo_without_caller_memo(&handler, &network_identifier, &operations, &pub_key)) - .collect(); - - assert!( - memos.iter().all(|memo| *memo == serde_json::json!(0)), - "expected a deterministic memo of 0 on every reconstruction, got {memos:?}" - ); + .clone(); + assert_eq!(memo, serde_json::json!(0), "expected a memo of 0"); } }