chore: stable change #6772
chore: stable change #6772
Conversation
Test Results (Integration tests) 2 files + 2 11 suites +11 40m 36s ⏱️ + 40m 36s For more details on these failures, see this check. Results for commit 3611a0e. ± Comparison against base commit 99ec71d. ♻️ This comment has been updated with latest results. |
Test Results (CI) 3 files 129 suites 35m 6s ⏱️ Results for commit 3611a0e. ♻️ This comment has been updated with latest results. |
WalkthroughThis pull request makes extensive adjustments across the repository. The changes update the Rust toolchain references in numerous CI and build configurations from a specific nightly version to stable. In addition, the CI environment has been refined with new environment variables for handling nightly builds. Numerous application modules and core components have been refactored for improved string length handling, error formatting, lifetime elisions, and simplified error mapping. Finally, new FFI functions were introduced, and minor cosmetic modifications were applied across tests and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant CFG as Config Files
participant Build as Build System
participant Test as Test Runner
CI->>CFG: Load toolchain config (stable, nightly_toolchain, etc.)
CFG-->>CI: Return updated environment variables
CI->>Build: Set environment & run ci-clippy with new alias
Build->>CI: Build binaries using stable toolchain
CI->>Test: Execute test jobs with updated toolchain settings
Test-->>CI: Return test results
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
.github/workflows/coverage.yml (1)
33-36: 💡 Verification agent❓ Verification inconclusive
Inconsistent Toolchain in Rust Toolchain Action
The step using thedtolnay/rust-toolchain@masteraction (lines 33–36) still specifiestoolchain: nightly. Since the objective is to use stable versions across the project, please update it to usestablefor consistency.A proposed diff change:
- - uses: dtolnay/rust-toolchain@master - with: - toolchain: nightly + - uses: dtolnay/rust-toolchain@master + with: + toolchain: stable
Action Required: Update Rust Toolchain to Stable in Coverage Workflow
The configuration in
.github/workflows/coverage.yml(lines 33–36) currently specifiestoolchain: nightly, which conflicts with our objective of using stable versions across the project. Please update this step to usestableinstead.
- File:
.github/workflows/coverage.yml(lines 33–36)- Proposed diff:
- - uses: dtolnay/rust-toolchain@master - with: - toolchain: nightly + - uses: dtolnay/rust-toolchain@master + with: + toolchain: stablecomms/core/src/connection_manager/common.rs (1)
144-150: 💡 Verification agent🧩 Analysis chain
Changed string length check from bytes to characters
The change modifies how the user agent length is calculated from byte count to character count. This creates a potential inconsistency with the configuration parameter name
max_user_agent_byte_lengthwhich still suggests it's limiting bytes, not characters.This could create issues with UTF-8 encoded strings where character count is lower than byte count. For example, a string with many non-ASCII characters would pass this check even if it exceeds the byte limit.
🏁 Script executed:
#!/bin/bash # Look for other similar checks and the definition of max_user_agent_byte_length rg "max_user_agent_byte_length" -A 2 -B 2Length of output: 1569
User agent length check mismatch: resolving byte vs. character count discrepancy
The current implementation in
comms/core/src/connection_manager/common.rsuses.len()(which returns the character count) for validating the user agent, while the configuration parametermax_user_agent_byte_length(defined incomms/core/src/peer_validator/config.rs) expects a byte count. This discrepancy means that user agents containing multibyte UTF-8 characters might bypass the intended byte limit.Action Items:
- Update the length check: Change the check to use
user_agent.as_bytes().len()if the intended limit is indeed in bytes.- Or update the configuration/documentation: If a character-based limit is preferred, adjust both the configuration name and its documentation accordingly.
🧹 Nitpick comments (14)
comms/dht/examples/propagation/prompt.rs (1)
90-90: Improved case-insensitive string comparisonThe change to use
eq_ignore_ascii_case(&'Y')overto_ascii_uppercase() != 'Y'is a good refinement. This approach more clearly expresses the intent of performing a case-insensitive comparison and avoids unnecessary string allocation.base_layer/wallet/src/connectivity_service/test.rs (1)
131-131: Good simplification of test assertionThe assertion has been simplified by removing the unnecessary mapping through
convert::identity. Both expressions achieve the same result, but the new version is more concise and easier to understand.buildtools/docker/base_node.Dockerfile (1)
7-7: Consider using COPY instead of ADD.Docker best practices recommend using COPY instead of ADD for simple file copying, as ADD has additional features that can lead to unexpected behavior.
-ADD . /minotari_node +COPY . /minotari_node🧰 Tools
🪛 Hadolint (2.12.0)
[error] 7-7: Use COPY instead of ADD for files and folders
(DL3020)
applications/minotari_app_utilities/src/parse_miner_input.rs (1)
108-149: Improved code style by removing unnecessaryreturnkeywords.The change removes explicit
returnkeywords from the match expression, making the code more idiomatic Rust. In Rust, the last expression in a block is implicitly returned, so explicitreturnstatements are only necessary for early returns. This refactoring improves readability while maintaining the exact same functionality.base_layer/core/src/proof_of_work/lwma_diff.rs (1)
105-105: Cleaner error handling with the?operator!The code now uses the
?operator for more concise error propagation, which improves readability while maintaining the same functionality as the previous explicit match statement. This change follows Rust idioms for handlingOptiontypes.applications/minotari_node/src/table.rs (1)
33-33: Good simplification of lifetime parametersThe change from
impl<'t, 's> Table<'t, 's>toimpl<'t> Table<'t, '_>appropriately uses an anonymous lifetime ('_) for the second lifetime parameter. This simplifies the implementation while maintaining the same functionality, as the second lifetime isn't directly referenced in method signatures.base_layer/core/src/transactions/transaction_protocol/transaction_initializer.rs (1)
112-112: Good optimization: Box the builder field to reduce stack usageBoxing the
SenderTransactionInitializerin theBuildErrorstruct helps manage memory more efficiently, especially if the builder is large. This change moves the potentially large builder struct from the stack to the heap.base_layer/wallet_ffi/src/lib.rs (1)
1530-1530: Consider using clearer doc comment style.Using a long line of punctuation in doc comments may hinder readability and clutter up the code. A shorter, more descriptive separator or standardized header comment might be clearer.
scripts/test_in_docker.sh (1)
6-6: Remove or document the unused variableThe
TOOLCHAIN_VERSIONvariable is defined but not used anywhere in the script, as flagged by the static analysis tool.Either remove this variable if it's not needed, or add a comment explaining its purpose if it's being kept for future use or reference:
-TOOLCHAIN_VERSION=stable +# TOOLCHAIN_VERSION is used for documentation purposes to track which Rust version this container uses +TOOLCHAIN_VERSION=stableAlternatively, if it's not needed:
-TOOLCHAIN_VERSION=stable🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 6-6: TOOLCHAIN_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
base_layer/wallet/tests/transaction_service_tests/service.rs (2)
497-500: More concise error handling with?operatorThe change replaces nested match statements with the
?operator, which provides a more elegant way to handle theOptiontype. This improves readability and reduces code complexity while maintaining the same functionality.This simplification could be extended to other similar error handling patterns in the codebase to improve consistency.
514-515: More concise error handling with?operatorSimilar to the previous function, this change replaces a nested match statement with the more idiomatic
?operator, making the code cleaner and more maintainable.This pattern of using the
?operator forOptionhandling is a standard Rust idiom now available in stable Rust, and could be applied consistently throughout the codebase..github/workflows/ci.yml (3)
16-24: Environment Variables Update and Consistency CheckThe CI environment variables have been updated to set the primary
toolchainto stable and introducenightly_toolchainandnightly_test_toolchain. Also, the new variableRUSTUP_PERMIT_COPY_RENAMEhas been added. Please verify that using nightly toolchains in certain jobs (e.g. clippy and testing) is intentional given the overall objective to move toward stable versions. This might require documentation or further clarification if the intent is to use stable builds exclusively.
43-46: Clippy Job Toolchain ReferenceThe clippy job now uses the environment variable
${{ env.nightly_toolchain }}for its toolchain. Ensure that using a nightly build for clippy (and subsequent formatting/linting steps) fits the long‐term goal of stability. If these steps are to eventually transition to stable releases, consider planning an update or adding comments to clarify the current rationale.
225-228: Nightly Test Toolchain in Test JobThe test job’s toolchain is set to use
${{ env.nightly_test_toolchain }}. Given the PR objective to transition to stable versions, please confirm whether it is necessary to continue running tests with a nightly toolchain. If not, consider updating the test job to use the stable toolchain or adding documentation to explain why nightly is still required in this context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
.cargo/config.toml(1 hunks).github/workflows/build_binaries.json(1 hunks).github/workflows/build_binaries.yml(1 hunks).github/workflows/build_dockers.yml(1 hunks).github/workflows/build_dockers_workflow.yml(1 hunks).github/workflows/ci.yml(7 hunks).github/workflows/coverage.yml(1 hunks).github/workflows/integration_tests.yml(5 hunks)applications/minotari_app_grpc/src/authentication/basic_auth.rs(2 hunks)applications/minotari_app_utilities/src/parse_miner_input.rs(2 hunks)applications/minotari_console_wallet/src/automation/commands.rs(1 hunks)applications/minotari_node/src/commands/cli.rs(1 hunks)applications/minotari_node/src/lib.rs(1 hunks)applications/minotari_node/src/table.rs(1 hunks)base_layer/common_types/src/types/bullet_rangeproofs.rs(1 hunks)base_layer/core/src/base_node/proto/response.rs(0 hunks)base_layer/core/src/base_node/state_machine_service/state_machine.rs(0 hunks)base_layer/core/src/chain_storage/horizon_data.rs(0 hunks)base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs(2 hunks)base_layer/core/src/consensus/consensus_encoding/hashing.rs(2 hunks)base_layer/core/src/covenants/macros.rs(0 hunks)base_layer/core/src/proof_of_work/difficulty.rs(0 hunks)base_layer/core/src/proof_of_work/lwma_diff.rs(1 hunks)base_layer/core/src/transactions/transaction_components/transaction_output.rs(0 hunks)base_layer/core/src/transactions/transaction_components/unblinded_output.rs(0 hunks)base_layer/core/src/transactions/transaction_components/wallet_output.rs(0 hunks)base_layer/core/src/transactions/transaction_protocol/transaction_initializer.rs(2 hunks)base_layer/key_manager/src/mnemonic_wordlists.rs(7 hunks)base_layer/mmr/src/merkle_proof.rs(1 hunks)base_layer/mmr/src/sparse_merkle_tree/node.rs(3 hunks)base_layer/mmr/src/sparse_merkle_tree/tree.rs(1 hunks)base_layer/p2p/src/initialization.rs(0 hunks)base_layer/p2p/src/services/liveness/service.rs(2 hunks)base_layer/wallet/src/connectivity_service/test.rs(1 hunks)base_layer/wallet/src/output_manager_service/handle.rs(2 hunks)base_layer/wallet/src/output_manager_service/service.rs(3 hunks)base_layer/wallet/src/transaction_service/service.rs(0 hunks)base_layer/wallet/tests/transaction_service_tests/service.rs(2 hunks)base_layer/wallet_ffi/README.md(1 hunks)base_layer/wallet_ffi/src/lib.rs(1 hunks)base_layer/wallet_ffi/wallet.h(1 hunks)buildtools/docker/base_node.Dockerfile(1 hunks)buildtools/windows-dev-environment-notes.md(1 hunks)comms/core/src/connection_manager/common.rs(1 hunks)comms/core/src/connectivity/connection_pool.rs(1 hunks)comms/core/src/memsocket/mod.rs(1 hunks)comms/core/src/noise/socket.rs(1 hunks)comms/core/src/peer_manager/node_id.rs(1 hunks)comms/core/src/protocol/rpc/body.rs(1 hunks)comms/core/src/socks/client.rs(1 hunks)comms/core/src/test_utils/mocks/connectivity_manager.rs(1 hunks)comms/core/src/tor/control_client/commands/del_onion.rs(1 hunks)comms/core/src/tor/control_client/commands/protocol_info.rs(1 hunks)comms/core/src/transports/dns/tor.rs(1 hunks)comms/core/src/transports/predicate.rs(1 hunks)comms/dht/examples/propagation/prompt.rs(1 hunks)integration_tests/src/chat_ffi.rs(1 hunks)integration_tests/src/ffi/callbacks.rs(1 hunks)rust-toolchain.toml(1 hunks)scripts/test_in_docker.sh(1 hunks)
💤 Files with no reviewable changes (10)
- base_layer/core/src/transactions/transaction_components/unblinded_output.rs
- base_layer/core/src/transactions/transaction_components/wallet_output.rs
- base_layer/core/src/covenants/macros.rs
- base_layer/core/src/base_node/state_machine_service/state_machine.rs
- base_layer/core/src/base_node/proto/response.rs
- base_layer/core/src/chain_storage/horizon_data.rs
- base_layer/p2p/src/initialization.rs
- base_layer/core/src/transactions/transaction_components/transaction_output.rs
- base_layer/wallet/src/transaction_service/service.rs
- base_layer/core/src/proof_of_work/difficulty.rs
✅ Files skipped from review due to trivial changes (8)
- rust-toolchain.toml
- integration_tests/src/chat_ffi.rs
- comms/core/src/peer_manager/node_id.rs
- base_layer/core/src/consensus/consensus_encoding/hashing.rs
- applications/minotari_node/src/lib.rs
- base_layer/mmr/src/sparse_merkle_tree/node.rs
- base_layer/mmr/src/sparse_merkle_tree/tree.rs
- base_layer/wallet_ffi/wallet.h
🧰 Additional context used
🪛 Hadolint (2.12.0)
buildtools/docker/base_node.Dockerfile
[error] 7-7: Use COPY instead of ADD for files and folders
(DL3020)
🪛 actionlint (1.7.4)
.github/workflows/ci.yml
69-69: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
70-70: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
118-118: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
119-119: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
250-250: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
251-251: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
.github/workflows/integration_tests.yml
90-90: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
91-91: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
171-171: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
172-172: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
🪛 Shellcheck (0.10.0)
scripts/test_in_docker.sh
[warning] 6-6: TOOLCHAIN_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: cargo check with stable
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: clippy
🔇 Additional comments (53)
buildtools/windows-dev-environment-notes.md (1)
399-399: Good update to stable Rust channel!This change from nightly channel to stable channel aligns perfectly with the PR objective of utilizing stable versions instead of nightly builds.
applications/minotari_node/src/commands/cli.rs (1)
99-99:❓ Verification inconclusive
Changed string length calculation method
This change updates the function to use
len()instead ofas_bytes().len()to calculate the length of the longest line. For ASCII strings, these methods return the same value, but for strings with multi-byte characters (like emojis or non-ASCII characters), they would yield different results.While the PR suggests this is part of the stable change, this particular modification could potentially affect the display of non-ASCII characters. Please verify that the output remains correctly formatted when non-ASCII characters are present.
🏁 Script executed:
#!/bin/bash # Check if any text in the banner or other UI components includes non-ASCII characters grep -r --include="*.rs" -P "[\x80-\xFF]" applications/minotari_node/assets/ # Check if the surrounding code expects character count rather than byte count grep -A 10 -B 10 "len()" applications/minotari_node/src/commands/cli.rsLength of output: 5899
Verify Display Output for Non-ASCII Characters
The updated change now uses
len()(returning the byte count) instead ofas_bytes().len(). Since the maximum line is selected usingchars().count(), this introduces a potential inconsistency for non-ASCII strings—where multi-byte characters could cause the computed length (and subsequent scaling by division) not to match the visual display width.
The assets (e.g., in
applications/minotari_node/assets/tari_logo.rsandtari_banner.rs) contain non-ASCII characters, so it's important to ensure that the banner and other UI outputs remain correctly aligned when these characters are present.In the file
applications/minotari_node/src/commands/cli.rswithin the functionmultiline_find_display_length, the line:result = line.len();is responsible for calculating the length; please verify that dividing this byte-length by 2 (and applying the subsequent adjustment) yields the intended display width for strings that include multi-byte characters.
Please run additional manual or automated UI verification tests with non-ASCII content to ensure that the formatting remains as expected.
base_layer/wallet_ffi/README.md (2)
135-136: Update successful: Switched from nightly to stable Rust toolchainThis change aligns perfectly with the PR objective to use stable Rust versions instead of nightly builds, which will enhance the stability and reliability of the project.
72-73:❓ Verification inconclusive
Consider updating the remaining nightly references in the document
While you've updated the Rust toolchain installation instructions to use stable instead of nightly, there are still references to nightly toolchain paths in other parts of the document:
~/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/aarch64-apple-ios/liband
~/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-ios/libFor complete consistency with the move to stable, consider updating these paths as well.
Also applies to: 85-86
🏁 Script executed:
#!/bin/bash # Check if there are other references to nightly that should be updated grep -r "nightly" --include="*.md" .Length of output: 2822
Update remaining nightly toolchain references in wallet_ffi README
While the main Rust toolchain installation instructions have been updated to use the stable toolchain, there are still lingering references to the nightly toolchain in this document. In particular:
Lines 72-73:
~/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/aarch64-apple-ios/libLines 85-86:
~/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-ios/libAdditionally, the command referencing the nightly toolchain (e.g.,
rustup component add rustfmt --toolchain nightly) should be reviewed for consistency.Please update these references to their stable equivalents to ensure complete consistency.
base_layer/mmr/src/merkle_proof.rs (2)
262-262: Error handling improvement: Replaced fold with try_foldThe change from
foldtotry_foldimproves error handling by allowing early termination if writing to the formatter fails. Sincewrite_strreturns aResult,try_foldis more appropriate thanfoldas it naturally handles the propagation of errors.
267-267: Error handling improvement: Replaced fold with try_foldSimilar to the previous change, using
try_foldhere improves error handling for the peak hashes formatting.comms/core/src/transports/dns/tor.rs (1)
83-83: Simplified error handling by removing unnecessary mappingThe removal of
.map_err(Into::into)simplifies the error handling by directly returning the result fromconvert_tcpip_multiaddr_to_socketaddr(&addr). This change streamlines the code while maintaining the same error propagation functionality, as the error type fromconvert_tcpip_multiaddr_to_socketaddris already compatible with the function's return type.Let's verify the error types to ensure they're compatible:
#!/bin/bash # Check the return type of convert_tcpip_multiaddr_to_socketaddr grep -r "fn convert_tcpip_multiaddr_to_socketaddr" --include="*.rs" .base_layer/common_types/src/types/bullet_rangeproofs.rs (1)
104-104: Simplified lifetime parameter in Visitor implementationThe change from
impl<'de> Visitor<'de> for RangeProofVisitortoimpl Visitor<'_> for RangeProofVisitoruses lifetime elision to simplify the code. This is a modern Rust practice that maintains the same functionality while making the code more concise and readable.This change is part of a broader effort to make the codebase more idiomatic and to facilitate the transition from nightly to stable Rust.
base_layer/p2p/src/services/liveness/service.rs (2)
38-38: Simplified import structure for better dependency management.The import of
DhtOutboundErrorhas been removed as it's no longer needed in the code.
251-257: Simplified error handling approach.The error mapping using
map_err(Into::<DhtOutboundError>::into)has been removed, which simplifies the error handling by directly using the question mark operator. This allows any error fromsend_direct_node_idto propagate naturally without an additional transformation, making the code more maintainable and easier to understand.base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs (2)
1644-1644: Streamlined error handling in database read operation.The
map_err(Into::into)transformation has been removed, allowing the original error type fromlmdb_getto propagate directly. This simplifies the error handling without changing the function's return type, as the method signature still returnsResult<Option<T>, ChainStorageError>.
1653-1653: Streamlined error handling in hash-to-height lookup.Similar to the previous change, the
map_err(Into::into)transformation has been removed, allowing the original error type fromlmdb_getto propagate directly. This makes the error handling more straightforward while maintaining the same function signature.buildtools/docker/base_node.Dockerfile (2)
3-3: Switched from nightly to stable Rust image.The build base image has been updated from a specific nightly version to the stable version, which aligns with the PR objective of using stable versions instead of nightly builds. This change will provide more reliable and consistent builds.
10-10: Updated toolchain reference to stable.The commented-out toolchain reference has been updated to use the stable version instead of a specific nightly version, maintaining consistency with the base image change.
comms/core/src/transports/predicate.rs (1)
44-44: Simplified lifetime parameter usage for improved readability.The implementation has been updated to use an elided lifetime (
'_) instead of a named lifetime parameter ('a). This change simplifies the code without affecting functionality, as the lifetime is inferred rather than explicitly stated, which is a common practice in modern Rust for improving code readability and maintenance.base_layer/key_manager/src/mnemonic_wordlists.rs (8)
11-11: Good conversion fromconsttostaticfor compatibility with stable Rust.This change from
consttostaticfor large arrays helps ensure compatibility with stable Rust versions by avoiding potential issues with large constants in memory.
144-144: Consistent type change with Chinese simplified wordlist.The change to static for the English wordlist maintains consistency with the pattern established for the Chinese simplified wordlist.
277-277: Correct migration pattern continued for French wordlist.This change follows the same pattern of converting from
consttostaticthat was applied to the previous wordlists.
410-410: Appropriately converted Italian wordlist to static.The conversion maintains consistency across all wordlists and is part of the larger effort to ensure compatibility with stable Rust.
543-543: Successfully migrated Japanese wordlist to static.This change is aligned with the project goal of using stable Rust versions instead of nightly builds.
676-676: Korean wordlist properly converted to static.The consistent application of this pattern across all wordlists demonstrates good attention to detail in the migration process.
809-809: Complete conversion of all wordlists with Spanish wordlist.The change from
consttostaticfor all seven wordlists has been consistently applied, completing the migration to stable Rust compatibility for this file.
1-939:✅ Verification successful
Verify memory implications of static vs const declarations for large arrays.
While this change is appropriate for compatibility with stable Rust, it's worth checking if there are any memory usage implications. Large static arrays have different memory behavior compared to const arrays, as const values are inlined at compile time while static values have a fixed memory location.
🏁 Script executed:
#!/bin/bash # Look for any usage of these wordlists in the codebase to understand potential impacts echo "Checking for usage of MNEMONIC_*_WORDS arrays in the codebase..." rg "MNEMONIC_.*_WORDS" --type rust -A 5 -B 5 echo -e "\nChecking for any tests or benchmarks measuring memory usage..." rg "memory|benchmark|bench" --type rust | grep -i "wordlist"Length of output: 28237
Using static for large wordlists is memory-efficient and appropriate 👍
From examining the codebase, these wordlists are used consistently for binary searches and lookups without modification, making them ideal candidates for
staticdeclarations. The change fromconsttostaticis positive for both compatibility and memory usage reasons:
These large arrays (2048 entries each) are referenced in multiple places across the codebase - in the mnemonic.rs file for word lookup/searches and in the wallet_ffi library.
Using
staticensures only one instance of each array exists in memory with a fixed address, rather than potentially inlining the large arrays at each use site (which could happen withconst).The read-only nature of these wordlists makes them perfect for
staticdeclarations.No memory benchmarks specifically targeting these wordlists were found, but the change should provide better memory efficiency for large data structures like these.
base_layer/core/src/transactions/transaction_protocol/transaction_initializer.rs (1)
545-546: Consistent implementation with boxed builder fieldThis correctly updates the error creation to align with the new
BuildErrorstruct definition, properly boxingselfwhen creating the error. This maintains consistency with the struct declaration change above..github/workflows/coverage.yml (1)
14-15: Update Environment Variable for Toolchain
The environment variable for the toolchain is now set tostablein line 14, which is aligned with the PR objective..github/workflows/build_dockers_workflow.yml (1)
14-18: Stable Default for Toolchain Input
The default value for thetoolchaininput has been updated tostable(line 17), which is consistent with the intended move away from nightly builds..github/workflows/build_binaries.json (1)
1-9: Rust Toolchain Configuration Updated to Stable
All Rust configurations in this JSON file have been updated to use"rust": "stable". This ensures that builds for all targets are now using the stable release..cargo/config.toml (1)
8-11: Updated ci-clippy Alias for Better Lint Control
The aliasci-clippyhas been updated (line 10) to include the suppression flag-- -A clippy::literal_string_with_formatting_args. This adjustment should help avoid spurious warnings from clippy..github/workflows/build_binaries.yml (2)
32-33: Stable Toolchain in Workflow Environment
The environment variabletoolchainhas been updated tostable(line 32), ensuring that the workflow adopts the stable Rust release. This change is in line with the overall PR objective.
185-191: Consistent Rust Toolchain Setup
In theSetup Rust toolchainstep (lines 185–191), the toolchain is set via${{ matrix.builds.rust }}. With the changes made in the build configurations, this should now resolve tostable. Please double-check that no lingering references to nightly remain..github/workflows/build_dockers.yml (1)
51-51: Switch to stable Rust toolchain aligns with project goalsThis change from a specific nightly version to the stable Rust toolchain is appropriate and aligns with the PR objective of enhancing stability by using well-tested releases instead of nightly builds.
scripts/test_in_docker.sh (1)
5-5: Switch to stable Docker image is appropriateChanging the Docker image to use the stable version instead of a nightly build aligns with the PR's objective to move away from nightly builds and improve stability.
comms/core/src/protocol/rpc/body.rs (1)
191-191: Code simplification improves readabilityThe implementation has been simplified by removing the unnecessary mapping operation. This is a good improvement that maintains the same functionality while making the code more concise and easier to understand.
Since
self.0is already of typeOption<Bytes>(as defined on line 152), the explicit mapping withmap(Bytes::from)was redundant.comms/core/src/memsocket/mod.rs (1)
292-292: Lifetime elision simplifies codeThis change from an explicit named lifetime parameter to an elided (anonymous) lifetime is a good practice that simplifies the code while maintaining the same functionality. This is part of idiomatic Rust, where elided lifetimes are preferred when they're not needed elsewhere in the implementation.
comms/core/src/test_utils/mocks/connectivity_manager.rs (1)
253-257: Error handling simplification: Removed unnecessary error conversion.The removal of
.map_err(Into::into)simplifies the error handling by directly returning the original error type from theok_oroperation. This is a positive change as it eliminates an unnecessary error conversion step while maintaining the same functionality.integration_tests/src/ffi/callbacks.rs (1)
335-336:❓ Verification inconclusive
Added safety warning suppression due to migration to stable Rust.
The addition of
#[allow(static_mut_refs)]suppresses warnings related to the use of mutable static references, which is necessary for using this singleton pattern with the stable Rust toolchain.While the singleton pattern implementation using static mutable references works, consider if this could be refactored to use safer alternatives in the future:
🏁 Script executed:
#!/bin/bash # Look for other singleton implementations in the codebase that might use safer patterns echo "Searching for potential safer singleton patterns..." rg -A 2 -B 2 "once_cell|lazy_static|parking_lot|Mutex|RwLock" --type rustLength of output: 67106
Attention: Consider Safer Singleton Pattern Alternatives
The current implementation in
integration_tests/src/ffi/callbacks.rsthat uses#[allow(static_mut_refs)]to support a singleton via mutable static references is working as intended with stable Rust. However, our search across the codebase shows that many modules already use safer alternatives (for example, patterns based ononce_cell,lazy_static, and synchronization primitives likeMutex/RwLock). Although this implementation remains acceptable in the integration tests context, please take a moment to evaluate whether refactoring it to follow one of these established, safer patterns might benefit long-term maintainability and thread safety—especially if similar patterns get used outside of test code.
- Location:
integration_tests/src/ffi/callbacks.rs, lines 335-336- Suggestion: Consider adopting alternatives such as
once_cellorlazy_staticto avoid mutable static references in future enhancements.comms/core/src/noise/socket.rs (1)
586-587: Error handling simplification: Removed unnecessary error conversion.The
.map_err(Into::into)call has been removed, simplifying the error handling by directly returning the result ofself.socket.flush().await. This change is part of a broader effort to streamline error handling across the codebase while transitioning from nightly to stable Rust.comms/core/src/tor/control_client/commands/protocol_info.rs (1)
44-44: Simplified lifetime parameter - good improvementThis change replaces a generic lifetime parameter
'awith an anonymous lifetime'_'in theTorCommandtrait implementation forProtocolInfo. This simplifies the code by using the Rust compiler's lifetime elision rules, making the code more concise while maintaining the same functionality.applications/minotari_console_wallet/src/automation/commands.rs (1)
1369-1369:✅ Verification successful
Added Box allocation for TransactionOutput
This change wraps
embedded_outputin aBoxbefore passing it toUseOutput::AsProvided, aligning with changes to theUseOutputenum which now expects a boxedTransactionOutputrather than a direct value. This modification ensures consistent memory management of potentially large transaction outputs throughout the codebase.
🏁 Script executed:
#!/bin/bash # Check for the definition of UseOutput enum to confirm the type change ast-grep --pattern $'enum UseOutput { $$$ AsProvided(Box<TransactionOutput>), $$$ }'Length of output: 91
I'm not seeing the expected output from the initial
ast-grepcommand. It might be that the pattern didn't match due to formatting differences or the location of the definition. To gain more confidence, please run the following additional shell scripts:Once these commands produce output confirming that the
UseOutput::AsProvidedvariant now expects aBox<TransactionOutput>, we can confidently conclude that wrappingembedded_outputinBox::new(...)is correct. If the output still does not match expectations, please verify manually that the enum is updated accordingly.Verify the Box Allocation Change for
TransactionOutput
- Ensure that the
UseOutputenum definition has been updated so that theAsProvidedvariant now expects a boxedTransactionOutput.- Confirm that wrapping
embedded_outputwithBox::new(...)aligns with this type change.Please run the commands above and verify the output.
🏁 Script executed:
#!/bin/bash echo "Searching for 'enum UseOutput' definition in the repository..." rg -C5 "enum\s+UseOutput" . echo "Searching for the specific variant pattern 'AsProvided(Box<TransactionOutput>)'..." rg -n "AsProvided\(Box<TransactionOutput>\)" .Length of output: 1488
Box Allocation Update Verified
The update in
applications/minotari_console_wallet/src/automation/commands.rswhereembedded_outputis wrapped usingBox::new(...)to match the updated enum variant inUseOutputhas been verified. Our search confirmed that theAsProvidedvariant now expects aBox<TransactionOutput>, as seen in the output of the following file:
- File:
base_layer/wallet/src/output_manager_service/service.rs
- Variant:
AsProvided(Box<TransactionOutput>)No further modifications are needed.
comms/core/src/connectivity/connection_pool.rs (1)
67-67: Improved is_connected check with more modern syntaxThe change replaces a likely
map_orpattern withis_some_and, which is more idiomatic and readable. This approach streamlines the code by combining the existence check and predicate application into a single method call.The logic remains the same: only return true if both the status indicates connected AND the underlying connection exists and is in a connected state.
comms/core/src/tor/control_client/commands/del_onion.rs (1)
40-40: Lifetime elision simplification looks good.This is a positive change that simplifies the code by using Rust's lifetime elision features. Replacing the explicit lifetime parameter
<'a>with the anonymous lifetime<'_>allows the Rust compiler to infer the appropriate lifetime. This maintains the same behavior while reducing verbosity and improving readability.applications/minotari_app_grpc/src/authentication/basic_auth.rs (2)
75-75: Consistent string length calculation in struct initialization.This change ensures consistency with the validation check above, using character count rather than byte count when storing the username length. Good for maintaining code consistency.
56-56:❓ Verification inconclusive
String length calculation now uses character count instead of byte count.
This change improves handling of non-ASCII characters by using the string's character count (
user_name.len()) rather than byte count (user_name.as_bytes().len()). This is more consistent with how users perceive string length.
🏁 Script executed:
#!/bin/bash # Verify that there's consistent use of string length calculation across the codebase # Check how many other places might be using .as_bytes().len() for string length checks echo "Places still using .as_bytes().len() for length checks:" rg '\.as_bytes\(\)\.len\(\).*MAX_.*LEN' --countLength of output: 170
Attention: Verified use of character count for string length calculation
The change in
applications/minotari_app_grpc/src/authentication/basic_auth.rscorrectly usesuser_name.len()to count characters rather than bytes, which improves non-ASCII handling. A repository-wide search for.as_bytes().len()in contexts comparing againstMAX_...LENproduced no matches, suggesting consistency across the codebase. However, please manually verify that all string length validations follow this approach.
- File:
applications/minotari_app_grpc/src/authentication/basic_auth.rs(line 56)- Change: Uses
user_name.len()instead of byte-based calculation- Observation: No other occurrences of the old pattern were found via automated search
comms/core/src/socks/client.rs (2)
146-147: Improved username length calculation for better character handling.This change uses character count (
username.len()) instead of byte count (username.as_bytes().len()), which is more consistent with how users perceive string length and handles non-ASCII characters better. The SOCKS5 protocol specifies that usernames should be between 1 and 255 characters, so this is the correct approach.
152-153: Improved password length calculation for better character handling.Similar to the username length change, this ensures consistency in how string lengths are calculated throughout the codebase. This approach is more user-friendly for non-ASCII characters.
.github/workflows/integration_tests.yml (3)
30-31: Appropriate transition to stable Rust toolchain while preserving nightly option.This change aligns with the PR objective of transitioning to stable Rust versions. Setting the default toolchain to stable while keeping the specific nightly version in a separate variable is a good approach that allows flexibility where nightly features might still be needed.
68-68: Consistent updates to use nightly toolchain where needed.The workflow has been properly updated to reference the new
nightly_toolchainvariable consistently throughout all the steps that need it. This maintains the existing functionality while supporting the transition to stable Rust.Also applies to: 96-96, 102-102, 150-150, 178-178, 184-184
90-93:❓ Verification inconclusive
Potential issue with cache keys using undefined runner property.
The static analysis tool detected that
runner.cpu-modelmight not be a defined property in the runner object, which could cause issues with cache key generation.If this property is custom or provided by your self-hosted runner setup, you can safely ignore this warning. If not, consider modifying the cache keys to use only defined properties.
Also applies to: 171-174
🏁 Script executed:
#!/bin/bash # Check other workflows to see if they use the same property echo "Checking if other workflows use the cpu-model property:" rg "runner.cpu-model" .github/workflows/Length of output: 3620
Action required: Verify custom runner property usage for cache keys
The cache keys in both the integration tests (lines 90–93 and 171–174) and the CI workflows use
runner.cpu-model. We verified that this property is used consistently across the workflows, which suggests it may be a custom property provided by your self-hosted runner setup. Please confirm that thecpu-modelproperty is indeed defined on your runners. If not, update the cache keys to use only standard, defined runner properties (e.g.,runner.os) to prevent caching issues.🧰 Tools
🪛 actionlint (1.7.4)
90-90: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
91-91: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
base_layer/wallet/src/output_manager_service/handle.rs (2)
300-308: Boxing large tuple to optimize memory usage.The
EncumberAggregateUtxovariant now uses a boxed tuple instead of directly embedding it in the enum. This is a good optimization that reduces the size of theOutputManagerResponseenum on the stack by moving the large tuple data to the heap.The change aligns well with the PR objective of providing stable changes, as it helps prevent potential stack overflows when dealing with large data structures.
890-909: Pattern matching updated correctly for boxed tuple.The pattern matching logic has been properly updated to handle the boxed tuple. The tuple is dereferenced with
*valuesand then destructured into its components. This change correctly aligns with the modification to the enum variant definition.The implementation ensures that the code functionality remains unchanged while benefiting from the memory optimization.
base_layer/wallet/src/output_manager_service/service.rs (3)
298-298: Response mapping updated to use Box.The response mapping for
EncumberAggregateUtxois now wrapping the tuple in aBox, which aligns with the updated enum variant in the handle.rs file. This ensures consistency across the codebase.This change is part of the memory optimization strategy to reduce stack usage for large data structures.
1342-1342: Updated clone and dereference pattern for boxed TransactionOutput.The code now properly dereferences the boxed
TransactionOutputwith*val.clone()when handling theUseOutput::AsProvidedvariant. This maintains the expected behavior while working with the new boxed type.This change correctly adapts the code to work with the boxed value while preserving the original functionality.
3463-3463: Boxing TransactionOutput in UseOutput enum.The
UseOutput::AsProvidedvariant now stores a boxedTransactionOutputinstead of directly embedding it. This is consistent with the other boxing changes and helps reduce the size of the enum on the stack.This change completes the optimization strategy by ensuring all large data structures in the relevant enums are boxed, improving overall memory efficiency.
| key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | ||
| restore-keys: | | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Caching Key Construction: Remove Undefined Runner Property
The caching keys in the clippy job include the expression ${{ runner.cpu-model }}, but static analysis indicates that the cpu-model property is not defined in the runner context. This could lead to unintended cache key values.
Consider removing ${{ runner.cpu-model }} from the key. For example:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small
+ key: tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-smallAnd similarly update the restore-keys accordingly.
Remove Undefined Runner Property in Caching Keys
After verifying the code snippet in .github/workflows/ci.yml and consulting the GitHub Actions runner context properties, it’s confirmed that the property runner.cpu-model is not defined. This means that its inclusion in the cache key and restore keys leads to unpredictable key values. Please remove ${{ runner.cpu-model }} from all caching keys. For example:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small
+ key: tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-smallAnd update the restore keys similarly:
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly
+ tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small
+ tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}
+ tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightlyPlease update these sections accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| restore-keys: | | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_toolchain }}-nightly | |
| key: tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| restore-keys: | | |
| tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ env.nightly_toolchain }}-nightly |
🧰 Tools
🪛 actionlint (1.7.4)
69-69: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
70-70: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
| key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | ||
| restore-keys: | | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Machete Job Caching Keys Need Revision
The caching keys in the machete job also reference ${{ runner.cpu-model }} when constructing the keys. As with the clippy job, this property is not defined in the runner object. I recommend removing it for more reliable caching keys. For example:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small
+ key: tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-smallAnd update the corresponding restore-keys similarly.
Fix Caching Key by Removing Undefined runner.cpu-model
In .github/workflows/ci.yml (lines 118–122), the caching keys for the machete job include ${{ runner.cpu-model }}, which isn’t a defined property in the GitHub Actions runner context. This can lead to unreliable cache keys. Please update the keys by removing this reference. For example:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small
+ key: tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-smallAnd update the corresponding restore keys similarly:
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}
- tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable
+ tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small
+ tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}
+ tari-${{ runner.os }}-${{ env.toolchain }}-stable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | |
| restore-keys: | | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-stable | |
| key: tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | |
| restore-keys: | | |
| tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-small | |
| tari-${{ runner.os }}-${{ env.toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }} | |
| tari-${{ runner.os }}-${{ env.toolchain }}-stable |
🧰 Tools
🪛 actionlint (1.7.4)
118-118: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
119-119: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
| key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.target }} | ||
| restore-keys: | | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.network }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.toolchain }}-nightly | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.network }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }} | ||
| tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Test Job Caching Keys Adjustment
In the test job, the caching keys again include ${{ runner.cpu-model }}. Since this property is not defined, remove it to ensure consistent caching behavior. For example:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.target }}
+ key: tari-${{ runner.os }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.target }}And make similar adjustments to the restore-keys that follow.
Fix Required: Update Caching Keys to Remove Undefined Variables
In the file .github/workflows/ci.yml (lines 250–254), the caching keys include the interpolation ${{ runner.cpu-model }}, which is not defined in GitHub Actions. This can lead to inconsistent caching behavior. Please remove this reference from both the key and the subsequent restore-keys sections.
For example, change:
- key: tari-${{ runner.os }}-${{ runner.cpu-model }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.target }}
+ key: tari-${{ runner.os }}-${{ env.nightly_test_toolchain }}-stable-${{ hashFiles('**/Cargo.lock') }}-${{ matrix.tari_target_network.target }}And update the restore keys in a similar fashion.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
250-250: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
251-251: property "cpu-model" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
Description
change Tari to use stable and not nightly
Summary by CodeRabbit