1.4.3 - Local

[tiljrd]

No description provided.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Error The condition on line 260 checks if "mev_type" == constants.MOCK_MEV_TYPE but should check result["mev_type"] == constants.MOCK_MEV_TYPE. This will always evaluate to false, making the validation ineffective. "mev_type" == constants.MOCK_MEV_TYPE and input_args["participants"][0]["cl_type"] != constants.CL_TYPE.lighthouse ): fail( "Mock mev is only supported if the first participant is lighthouse client, please use a different client or set mev_type to 'flashbots', 'mev-rs' or 'commit-boost' or make the first participant lighthouse" ) Breaking Change The removal of spamoor_blob_params and related functionality represents a significant breaking change that could affect existing configurations and deployments without proper migration guidance. ) Version Mismatch The version in version.txt shows 5.0.1 but the PR title indicates 1.4.3 - Local, creating confusion about the actual version being released. 5.0.1

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix string comparison bug The condition checks for string equality with "mev_type" instead of checking the actual mev_type value. This will always evaluate to False since the string "mev_type" is never equal to constants.MOCK_MEV_TYPE. src/package_io/input_parser.star [259-265] if ( - "mev_type" == constants.MOCK_MEV_TYPE + result["mev_type"] == constants.MOCK_MEV_TYPE and input_args["participants"][0]["cl_type"] != constants.CL_TYPE.lighthouse ): fail( "Mock mev is only supported if the first participant is lighthouse client, please use a different client or set mev_type to 'flashbots', 'mev-rs' or 'commit-boost' or make the first participant lighthouse" ) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical bug where a string literal "mev_type" is compared instead of the variable result["mev_type"]. This logic error causes the validation check to always be false, preventing it from ever executing as intended.	High
	Add null check for time value Add validation to ensure osaka_time is not None before assigning it to the configuration. This prevents potential runtime errors when the time value is undefined. src/spamoor/spamoor.star [37-43] if ( "peerdas" in network_params.network or network_params.fulu_fork_epoch != constants.FAR_FUTURE_EPOCH ) and "blob" in spammer["scenario"]: if "config" not in spammer: spammer["config"] = {} - spammer["config"]["fulu_activation"] = osaka_time + if osaka_time is not None: + spammer["config"]["fulu_activation"] = osaka_time [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that osaka_time could be None and proposes adding a check to prevent potential issues. This improves the robustness of the code, which is a valid and helpful change.	Low
	Remove unnecessary format call The empty string in the format call will result in an empty command line argument when gcmode_archive is False. This creates an invalid command with an empty string argument that could cause geth to fail or behave unexpectedly. src/el/geth/geth_launcher.star [184] -"{0}".format("--state.scheme=hash" if gcmode_archive else ""), +"--state.scheme=hash" if gcmode_archive else "", [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 2 __ Why: The suggestion correctly points out that the .format() call is redundant. However, the proposed change is purely stylistic and does not fix the underlying potential issue of passing an empty string as a command-line argument, which was the main concern raised in the suggestion's description.	Low
General	Clarify nested ternary logic The nested ternary operator logic is confusing and may not handle all cases correctly. When gcmode_archive is True, it should use "--gcmode=archive", but the current logic might not work as expected for all combinations. src/el/geth/geth_launcher.star [216-221] -"--syncmode=full" -if network_params.network == constants.NETWORK_NAME.kurtosis -and not gcmode_archive -else "--syncmode=snap" -if not gcmode_archive -else "--gcmode=archive", +"--gcmode=archive" if gcmode_archive else ( + "--syncmode=full" if network_params.network == constants.NETWORK_NAME.kurtosis + else "--syncmode=snap" +), [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the nested ternary operator is complex and hard to read. The improved_code provides a much clearer and more maintainable equivalent by handling the primary condition (gcmode_archive) first, which significantly improves code readability and reduces the chance of logic errors.	Medium
	Improve error message context The error handling captures the curl exit code but doesn't provide meaningful context about what failed. Consider adding more descriptive error messages to help with debugging. src/network_launcher/shadowfork.star [43] -curl -s -f -o /shadowfork/latest_block.json "$URL" || { echo "Curl failed with exit code $?"; exit 1; } +curl -s -f -o /shadowfork/latest_block.json "$URL" || { echo "Failed to fetch block data from $URL (curl exit code: $?)"; exit 1; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that the error message for the curl command lacks context. Adding the $URL to the error message, as proposed, significantly improves debuggability by showing which URL failed to be fetched. This is a valuable improvement for maintainability.	Low
Update