diff --git a/.github/workflows/todo-format-check.yml b/.github/workflows/todo-format-check.yml new file mode 100644 index 0000000000..e2aa7355bf --- /dev/null +++ b/.github/workflows/todo-format-check.yml @@ -0,0 +1,23 @@ +name: TODO Format Check + +on: + workflow_dispatch: + pull_request: + merge_group: + push: + branches: + - main + +jobs: + check-todo-format: + name: "Check that TODO comments follow the correct format" + runs-on: warp-ubuntu-2204-x64-2x + permissions: + contents: read + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + + - name: Check TODO comment format + run: ./scripts/check-todo-format.sh diff --git a/Makefile.toml b/Makefile.toml index 330e6ed335..c0aace4b89 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -2,6 +2,7 @@ description = "Run all fast checks" dependencies = [ "crate-names", + "todo-format", "ruff-format", "ruff-check", "fmt", @@ -17,6 +18,7 @@ dependencies = [ description = "Run all checks" dependencies = [ "crate-names", + "todo-format", "ruff-format", "ruff-check", "fmt", @@ -85,6 +87,10 @@ args = [".github/"] command = "sh" args = ["scripts/check-crates-kebab-case.sh"] +[tasks.todo-format] +command = "sh" +args = ["scripts/check-todo-format.sh"] + [tasks.cargo-deny] description = "Run cargo deny" command = "cargo" diff --git a/crates/contract/README.md b/crates/contract/README.md index ee4325c58c..86cea2a3bb 100644 --- a/crates/contract/README.md +++ b/crates/contract/README.md @@ -323,7 +323,7 @@ The prospective node operator can retrieve that data from the web endpoint (`:80 The process of doing so is as follows: -1. The prospective participants set up their MPC inside their TEE environment (todo: [#550](https://github.com/near/mpc/issues/550), documentation to follow). +1. The prospective participants set up their MPC inside their TEE environment (TODO(#550): documentation to follow). 2. The prospective participants fetch their TEE related information from their logs. 3. The prospective participants add the `near_signer_public_key` from the web endpoint (`:8080/get_public_data`) as an access key to their node operator account, eligible for calling the MPC contract (`v1.signer` on mainnet or `v1.signer-prod.testnet` on testnet). Participants should provide sufficient funding to this key. 4. The prospective participants add the `near_responder_public_keys` from the web endpoint to a different account and provide sufficient funding to it. diff --git a/crates/contract/assets/launcher_docker_compose.yaml.template b/crates/contract/assets/launcher_docker_compose.yaml.template index 0e0569eb1f..050f25195c 100644 --- a/crates/contract/assets/launcher_docker_compose.yaml.template +++ b/crates/contract/assets/launcher_docker_compose.yaml.template @@ -2,7 +2,7 @@ version: '3.8' services: launcher: - image: nearone/mpc-launcher@sha256:4065f2fce41415962be92471a4e793ff5147b00b2784617c7e8098be2761a875 + image: nearone/mpc-launcher@sha256:7f155ebc928579ca269840d2e59f2e08e3c0cc1271c586f2e2b74bbda27ee8e1 container_name: launcher diff --git a/crates/contract/src/config.rs b/crates/contract/src/config.rs index 0546bcf81a..889c9e2f5e 100644 --- a/crates/contract/src/config.rs +++ b/crates/contract/src/config.rs @@ -24,7 +24,7 @@ const DEFAULT_FAIL_ON_TIMEOUT_TERA_GAS: u64 = 2; /// Prepaid gas for a `clean_tee_status` call const DEFAULT_CLEAN_TEE_STATUS_TERA_GAS: u64 = 10; /// Prepaid gas for a `cleanup_orphaned_node_migrations` call -/// todo: benchmark [#1164](https://github.com/near/mpc/issues/1164) +/// TODO(#1164): benchmark const DEFAULT_CLEANUP_ORPHANED_NODE_MIGRATIONS_TERA_GAS: u64 = 3; /// Prepaid gas for a `remove_non_participant_update_votes` call const DEFAULT_REMOVE_NON_PARTICIPANT_UPDATE_VOTES_TERA_GAS: u64 = 5; diff --git a/crates/contract/src/lib.rs b/crates/contract/src/lib.rs index 3706fcb5b9..0b058476a6 100644 --- a/crates/contract/src/lib.rs +++ b/crates/contract/src/lib.rs @@ -1183,8 +1183,7 @@ impl MpcContract { parameters.validate().unwrap(); - // TODO: https://github.com/near/mpc/issues/1087 - // Every participant must have a valid attestation, otherwise we risk + // TODO(#1087): Every participant must have a valid attestation, otherwise we risk // participants being immediately kicked out once contract transitions into running. let initial_participants = parameters.participants(); let tee_state = TeeState::with_mocked_participant_attestations(initial_participants); @@ -1507,7 +1506,7 @@ impl MpcContract { &mut self, destination_node_info: DestinationNodeInfo, ) -> Result<(), Error> { - // todo: require a deposit [#1163](https://github.com/near/mpc/issues/1163) + // TODO(#1163): require a deposit let account_id = Self::assert_caller_is_signer(); diff --git a/crates/contract/src/update.rs b/crates/contract/src/update.rs index 257e62608a..24ee3a2dfc 100644 --- a/crates/contract/src/update.rs +++ b/crates/contract/src/update.rs @@ -237,7 +237,7 @@ impl ProposedUpdates { /// Removes votes from accounts that are not participants. pub fn remove_non_participant_votes(&mut self, participants: &Participants) { // Note: This operation has quadratic time complexity. - // TODO issue [#1572](https://github.com/near/mpc/issues/1572) + // TODO(#1572): optimize quadratic time complexity let non_participants: Vec = self .vote_by_participant .keys() diff --git a/crates/contract/tests/sandbox/common.rs b/crates/contract/tests/sandbox/common.rs index 6e490eb801..8dac75179d 100644 --- a/crates/contract/tests/sandbox/common.rs +++ b/crates/contract/tests/sandbox/common.rs @@ -316,7 +316,7 @@ pub async fn submit_tee_attestations( node_id.account_id, "AccountId mismatch" ); - let attestation = Attestation::Mock(MockAttestation::Valid); // todo #1109, add TLS key. + let attestation = Attestation::Mock(MockAttestation::Valid); // TODO(#1109): add TLS key. let result = submit_participant_info( account, contract, diff --git a/crates/contract/tests/sandbox/tee_cleanup_after_resharing.rs b/crates/contract/tests/sandbox/tee_cleanup_after_resharing.rs index 2775f5ceb4..c6b84aa7e4 100644 --- a/crates/contract/tests/sandbox/tee_cleanup_after_resharing.rs +++ b/crates/contract/tests/sandbox/tee_cleanup_after_resharing.rs @@ -67,7 +67,7 @@ async fn test_tee_cleanup_after_full_resharing_flow() -> Result<()> { tls_public_key: bogus_ed25519_near_public_key(), account_public_key: Some(bogus_ed25519_near_public_key()), }; - let attestation = Attestation::Mock(MockAttestation::Valid); // todo #1109, add TLS key. + let attestation = Attestation::Mock(MockAttestation::Valid); // TODO(#1109): add TLS key submit_participant_info( &mpc_signer_accounts[0], &contract, diff --git a/crates/contract/tests/sandbox/upgrade_from_current_contract.rs b/crates/contract/tests/sandbox/upgrade_from_current_contract.rs index 349a86f512..e55d213b47 100644 --- a/crates/contract/tests/sandbox/upgrade_from_current_contract.rs +++ b/crates/contract/tests/sandbox/upgrade_from_current_contract.rs @@ -218,7 +218,7 @@ async fn test_invalid_contract_deploy() { dbg!(state); } -// TODO(#496) Investigate flakiness of this test +// TODO(#496): Investigate flakiness of this test #[tokio::test] async fn test_propose_update_contract_many() { let SandboxTestSetup { diff --git a/crates/contract/tests/sandbox/utils/consts.rs b/crates/contract/tests/sandbox/utils/consts.rs index 0c0910b8d7..3955797ca9 100644 --- a/crates/contract/tests/sandbox/utils/consts.rs +++ b/crates/contract/tests/sandbox/utils/consts.rs @@ -18,7 +18,7 @@ pub const ALL_SIGNATURE_SCHEMES: &[SignatureScheme; 4] = &[ /// we use this constant to attach a fixed amount to each call and detect if gas usage /// increases unexpectedly in the future. /// -/// TODO(#926) this gas was bumped from 22 to 34 in https://github.com/near/mpc/pull/1559. This +/// TODO(#926): this gas was bumped from 22 to 34 in https://github.com/near/mpc/pull/1559. This /// might be due to the high cost of `self.protocol_state = new_state` in the vote_reshard /// contract call. This needs to be investigated to understand why the increase was necessary. pub const GAS_FOR_VOTE_RESHARED: Gas = Gas::from_tgas(34); diff --git a/crates/devnet/src/terraform.rs b/crates/devnet/src/terraform.rs index 708c018ad6..1c560c7cc7 100644 --- a/crates/devnet/src/terraform.rs +++ b/crates/devnet/src/terraform.rs @@ -317,8 +317,7 @@ pub fn get_urls(name: &str, config: &ParsedConfig) -> Vec { "http://{}:8080", instance.nat_ip().unwrap_or_default() )); - // todo: display account_ids on the endpoint too, as this index might not be reliable. - // [(#712)](https://github.com/near/mpc/issues/712) + // TODO(#712): display account_ids on the endpoint too, as this index might not be reliable } } ret diff --git a/crates/mpc-attestation/src/attestation.rs b/crates/mpc-attestation/src/attestation.rs index 7b8a14e293..fa0e852a6a 100644 --- a/crates/mpc-attestation/src/attestation.rs +++ b/crates/mpc-attestation/src/attestation.rs @@ -75,7 +75,7 @@ impl Attestation { let accepted_measurements = [ include_measurements!("assets/tcb_info.json"), - // TODO Security #1433 - remove dev measurements from production builds after testing is complete. + // TODO(#1433): Security - remove dev measurements from production builds after testing is complete include_measurements!("assets/tcb_info_dev.json"), ]; diff --git a/crates/node/src/cli.rs b/crates/node/src/cli.rs index 620aced4f3..eda7588782 100644 --- a/crates/node/src/cli.rs +++ b/crates/node/src/cli.rs @@ -252,7 +252,7 @@ impl StartCmd { profiler::web_server::start_web_server(config.pprof_bind_address).await?; - // TODO (#1296) + // TODO(#1296): Decide if the MPC responder account is actually needed let respond_config = RespondConfig::from_parts(&config, &persistent_secrets); let backup_encryption_key_hex = match &self.backup_encryption_key_hex { diff --git a/crates/node/src/config.rs b/crates/node/src/config.rs index fd11ad54b7..d2d67e05a1 100644 --- a/crates/node/src/config.rs +++ b/crates/node/src/config.rs @@ -404,7 +404,7 @@ impl PersistentSecrets { ); let secrets = if let Some(secrets) = Self::maybe_get_existing(home_dir)? { tracing::debug!("p2p and near account secret key already exists. Using existing."); - // todo: consistent number of keys [(#534)](https://github.com/near/mpc/issues/534) + // TODO(#534): consistent number of keys secrets } else { tracing::debug!("p2p and near account secret key not found. Generating..."); @@ -505,7 +505,7 @@ pub struct RespondConfig { impl RespondConfig { pub fn from_parts(config: &ConfigFile, secrets: &PersistentSecrets) -> Self { - // TODO(#1296). cleanup. + // TODO(#1296): Decide if the MPC responder account is actually needed // updated as part PR #1270 as temporary solution. // using main account for responding. Self { diff --git a/crates/node/src/indexer/fake.rs b/crates/node/src/indexer/fake.rs index 758ff24d94..43bde08a46 100644 --- a/crates/node/src/indexer/fake.rs +++ b/crates/node/src/indexer/fake.rs @@ -513,7 +513,7 @@ impl FakeIndexerCore { contract.vote_reshared(account_id, reshared.key_event_id); } ChainSendTransactionRequest::StartKeygen(start) => { - // todo: timeout logic in fake indexer? + // TODO: timeout logic in fake indexer? let mut contract = contract.lock().await; contract.vote_start_keygen(account_id, start.key_event_id); } diff --git a/crates/node/src/indexer/handler.rs b/crates/node/src/indexer/handler.rs index e2a30fe8e9..bda7d1e6c2 100644 --- a/crates/node/src/indexer/handler.rs +++ b/crates/node/src/indexer/handler.rs @@ -151,7 +151,7 @@ async fn handle_message( let receipt = outcome.receipt.clone(); let execution_outcome = outcome.execution_outcome.clone(); - // TODO: this should improve once https://github.com/near/mpc/issues/950 is done + // TODO(#950): this should improve once the issue is resolved if let Some(next_receipt_id) = try_extract_next_receipt_id(&execution_outcome, mpc_contract_id) { diff --git a/crates/node/src/indexer/real.rs b/crates/node/src/indexer/real.rs index 90d1a3cf48..e6b0748089 100644 --- a/crates/node/src/indexer/real.rs +++ b/crates/node/src/indexer/real.rs @@ -72,7 +72,7 @@ pub fn spawn_real_indexer( let (txn_sender_sender, txn_sender_receiver) = oneshot::channel(); std::thread::spawn(move || { - // TODO(#1515); limit number of worker threads? Assume not as we don't want the node to fall behind. + // TODO(#1515): limit number of worker threads? Assume not as we don't want the node to fall behind let indexer_tokio_runtime = tokio::runtime::Builder::new_multi_thread() .enable_all() .build() diff --git a/crates/node/src/keyshare/permanent.rs b/crates/node/src/keyshare/permanent.rs index 82099dd956..d4cd0ba2d9 100644 --- a/crates/node/src/keyshare/permanent.rs +++ b/crates/node/src/keyshare/permanent.rs @@ -21,7 +21,7 @@ impl PermanentKeyshareData { } } - // [todo #1217](https://github.com/near/mpc/issues/1217): Move this to a separate crate, s.t. we are forced to use the constructor + // TODO(#1217): Move this to a separate crate, s.t. we are forced to use the constructor pub fn new(epoch_id: EpochId, keyshares: Vec) -> anyhow::Result { let is_consistent = keyshares.windows(2).all(|w| { w[0].key_id.epoch_id == w[1].key_id.epoch_id diff --git a/crates/node/src/p2p.rs b/crates/node/src/p2p.rs index f5068ea02e..6f94732f61 100644 --- a/crates/node/src/p2p.rs +++ b/crates/node/src/p2p.rs @@ -732,7 +732,7 @@ mod tests { sender1.wait_for_ready(2, &all_participants).await.unwrap(); for _ in 0..100 { - // todo: adjust test? + // TODO: adjust test? let domain_id = rand::thread_rng().gen(); let epoch_id = rand::thread_rng().gen(); let n_attempts = rand::thread_rng().gen::() % 100; diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index 25cd8612ec..32921f69a4 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -192,7 +192,7 @@ pub async fn monitor_attestation_removal( allowed_launcher_compose_hashes_in_contract: watch::Receiver>, mut tee_accounts_receiver: watch::Receiver>, ) -> anyhow::Result<()> { - // TODO: we should unify these conversions, will not be needed after https://github.com/near/mpc/issues/1246 + // TODO(#1246): we should unify these conversions, will not be needed after the issue is resolved let node_id = NodeId { account_id: node_account_id.clone(), tls_public_key: near_sdk::PublicKey::from_parts( diff --git a/crates/test-parallel-contract/src/lib.rs b/crates/test-parallel-contract/src/lib.rs index cea17fae4e..b4461b4af9 100644 --- a/crates/test-parallel-contract/src/lib.rs +++ b/crates/test-parallel-contract/src/lib.rs @@ -14,7 +14,7 @@ use sha2::{Digest, Sha256}; use std::collections::BTreeMap; use contract_interface::types::Bls12381G1PublicKey; -// TODO: all these types should come from mpc_contract https://github.com/near/mpc/issues/1057 +// TODO(#1057): all these types should come from mpc_contract #[derive(Serialize)] #[serde(rename_all = "PascalCase")] diff --git a/docs/migration-service.md b/docs/migration-service.md index a58f45243a..c093d0eb6c 100644 --- a/docs/migration-service.md +++ b/docs/migration-service.md @@ -302,7 +302,7 @@ pub struct NodeMigrations { } ``` -Additionally, the backup service will need to provide a TEE attestation similar to MPC nodes, which requires extending the contract to support attestation verification for backup services. (TODO issue [#947](https://github.com/near/mpc/issues/947)) +Additionally, the backup service will need to provide a TEE attestation similar to MPC nodes, which requires extending the contract to support attestation verification for backup services. (TODO(#947): Define attestation data for backup service) #### Backup Service Registration @@ -321,7 +321,7 @@ The backup service attestation registreation and verification would follow the s - Transaction signer's public key matches `account_public_key` via `env::signer_account_pk()` 7. Contract stores `TeeState` (containing attestation and all verification data) -> **Note**: Unlike MPC nodes which may need multiple attestations per operator, backup services [use](#todo) a simpler one-per-operator model. The `AccountId` remains the unique identifier, consistent with soft launch. +> **Note**: Unlike MPC nodes which may need multiple attestations per operator, backup services use a simpler one-per-operator model. The `AccountId` remains the unique identifier, consistent with soft launch. #### Backup Service TEE methods @@ -445,7 +445,7 @@ Both soft launch and hard launch implementations share common core components, w - Prometheus-style metrics (keyshare freshness, backup success/failure rates) - Operator dashboards for status visibility -### Todo +### Remaining Work See [(#949)](https://github.com/near/mpc/issues/949) - It is advised that the node operator grants access only to specific contract methods for the backup service and the node: [(#946)](https://github.com/near/mpc/issues/946) diff --git a/docs/securing_mpc_with_tee_design_doc.md b/docs/securing_mpc_with_tee_design_doc.md index 87c6263a58..d789ec726b 100644 --- a/docs/securing_mpc_with_tee_design_doc.md +++ b/docs/securing_mpc_with_tee_design_doc.md @@ -411,7 +411,7 @@ In contrast to the pre-TEE design, where those keys were generated by the operat The private key never leaves the CVM. While the public key can be exported from the CVM via an HTTP endpoint /public_data -The Operator will then register the node's account key as an additional access key for the node's near account, and provide it with the following permissions (TODO - add list of contract APIs) +The Operator will then register the node's account key as an additional access key for the node's near account, and provide it with the following permissions (TODO: add list of contract APIs) In addition - The node will generate multiple responder account keys as well. These are used to increase throughout when posting generated MPC signatures to the chain. diff --git a/pytest/tests/test_lost_assets.py b/pytest/tests/test_lost_assets.py index 982b4d36ae..ec94d40d2d 100644 --- a/pytest/tests/test_lost_assets.py +++ b/pytest/tests/test_lost_assets.py @@ -104,7 +104,7 @@ def test_cleanup_dead_node(lost_assets_cluster: MpcCluster): lost_assets_cluster.send_and_await_signature_requests(presignatures_available) -# Todo: [(#791)](https://github.com/near/mpc/issues/791) requires MPC node binary with enabled network-hardship-simulation feature +# TODO(#791): requires MPC node binary with enabled network-hardship-simulation feature @pytest.mark.no_atexit_cleanup def test_cleanup_lagging_node(lost_assets_cluster: MpcCluster): """ diff --git a/scripts/check-todo-format.sh b/scripts/check-todo-format.sh new file mode 100755 index 0000000000..510408911b --- /dev/null +++ b/scripts/check-todo-format.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Define extensions as an array for cleaner handling +CHECKED_EXTENSIONS=( + "*.rs" "*.py" "*.sh" "*.yml" "*.yaml" "*.md" +) + +# 1. git ls-files -z uses null-termination for space-safe filenames +# 2. xargs -0 reads those null-terminated strings +# 3. grep \btodo\b looks for the word "todo" specifically +# 4. The final grep -v -E filters out the two valid patterns +INVALID_TODOS=$(git ls-files -z "${CHECKED_EXTENSIONS[@]}" | \ + xargs -0 grep -HinE "\btodo\b" | \ + grep -vE ":.*(TODO\(#[0-9]+\):|TODO:)" || true) + +# Filter out the script itself and todo-format-check files from the results +SCRIPT_NAME=$(basename "$0") +INVALID_TODOS=$(echo "$INVALID_TODOS" | grep -v "$SCRIPT_NAME" | grep -v "todo-format-check.ya\?ml" || true) + +if [ -n "$INVALID_TODOS" ]; then + echo "❌ Found TODO comments not matching the required format" + echo "" + echo "Valid formats:" + echo " TODO(#1234): description" + echo " TODO: description" + echo "" + echo "Invalid lines found:" + echo "$INVALID_TODOS" + exit 1 +fi + +echo "✅ All TODO comments follow the required format" +exit 0 \ No newline at end of file diff --git a/tee_launcher/launcher.py b/tee_launcher/launcher.py index 495dc4d15e..8e5227c568 100644 --- a/tee_launcher/launcher.py +++ b/tee_launcher/launcher.py @@ -620,7 +620,7 @@ def get_manifest_digest( raise Exception(f"No tags found for image {docker_image.spec.image_name}") # We need an authorization token to fetch manifests. - # TODO this still has the registry hard-coded in the url. also, if we use a different registry, we need a different auth-endpoint. + # TODO: this still has the registry hard-coded in the url. also, if we use a different registry, we need a different auth-endpoint. token_resp = requests.get( f"https://auth.docker.io/token?service=registry.docker.io&scope=repository:{docker_image.name()}:pull" )