EFM and main branch Rosetta update - #71
Conversation
Remove previewnet references WIP go mods update
* update to flow-go v0.38.0-rc.2 * update imports --------- Co-authored-by: Bastian Müller <bastian@turbolent.com>
pin latest feature/efm-recovery Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
…le to make it more CI friendly and implement less code
|
Warning Rate limit exceeded@franklywatson has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes update continuous integration, build, and configuration files while removing various deprecated test and network configurations. Key updates include the introduction of an environment variable, upgrades to dependencies and the Go toolchain, revised documentation with updated command examples, and new Makefile targets for account management and fund transactions. Several files have been removed to improve testing practices and simplify network support. Additionally, the Rosetta CLI functionality has been extended with commands for creating derived accounts and transferring funds. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as Rosetta CLI
participant RS as Rosetta API Server
participant ES as External Signer
U->>CLI: Run rosetta_create_derived_account command
CLI->>RS: Preprocess transaction (via request_router)
RS-->>CLI: Return unsigned transaction data
CLI->>ES: Invoke external signing script
ES-->>CLI: Return signed transaction data
CLI->>RS: Combine and submit transaction
RS-->>CLI: Return transaction hash
CLI->>U: Display transaction receipt
Suggested reviewers
Poem
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 (
|
…efm-recovery-rosetta # Conflicts: # .github/workflows/ci.yml # go.mod # go.sum
franklywatson
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (26)
rosetta_handler.py (4)
39-50: Potential refactoring for shared network_identifier.
The code repeats the same "blockchain/network" fields. Consider centralizing these fields in a helper function or global constant to reduce duplication and risk of inconsistent references.
52-66: Keep payload generation consistent with future expansions.
The function is structured similarly to the preprocess and metadata steps. You might unify them under a single "build_request" approach to reduce repeated code.
68-97: Consider multi-signature scenarios.
Right now, only one signature is shown in "signatures". If multi-signature transactions become necessary, you may need to iterate over multiple signers or pass a list of signatures.
149-213: Duplicate logic with rosetta_create_derived_account.
Code for constructing, signing, and submitting is nearly identical. Abstract these steps into reusable helper methods to eliminate duplication and reduce bugs.cmd/genkey/genkey.go (1)
29-38: Conditional output structure is nicely implemented.
The code elegantly prints CSV or multiple lines. Consider adding a brief docstring or usage note so users understand which is which.script/flow.json (1)
37-56: Add Root Originator Accounts
The two new accounts, "root-originator-account-1" and "root-originator-account-2", are introduced with their keys and addresses. Hardcoding private keys is acceptable for test or local environments but be cautious in production.Makefile (1)
1-9: Centralize Makefile Configuration Variables
Introducing environment constants (ACCOUNT_KEYS_FILENAME, FLOW_JSON, FLOW_JSON_NETWORK, FLOW_JSON_SIGNER, FLOW_CLI_FLAGS, ROSETTA_ENV, ROSETTA_HOST_URL, and COMPILER_FLAGS) streamlines configuration across targets. This centralization aids maintainability.script/README.md (14)
5-7: Enhanced Description with Context
The updated guide description explains the testing and validation approach in the context of flow-go compatibility. For improved readability, consider breaking long sentences into two shorter ones.
31-34: Target Environment Configuration: Punctuation Enhancement
Consider adding a comma after “Before starting” to improve readability. For example:-Before starting ensure that variable constants in the project... +Before starting, ensure that variable constants in the project...🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: Possible missing comma found.
Context: ...## Configure target environment Before starting ensure that variable constants in the p...(AI_HYDRA_LEO_MISSING_COMMA)
36-40: First Time Environment Setup: Punctuation Suggestion
In the sentence on line 39, adding a comma after “If testing a live network” would improve clarity:-If testing a live network first time setup steps are only required if the network is new. +If testing a live network, first time setup steps are only required if the network is new.🧰 Tools
🪛 LanguageTool
[uncategorized] ~39-~39: Possible missing comma found.
Context: ...originator accounts. If testing a live network first time setup steps are only require...(AI_HYDRA_LEO_MISSING_COMMA)
41-48: Emulator Usage Instructions: Punctuation Improvement
In line 47, it is recommended to add a comma after “full state reset” for improved clarity:-...if the `flowdb/` directory is deleted for a full state reset then bootstrapping will need to be run again +...if the `flowdb/` directory is deleted for a full state reset, then bootstrapping will need to be run again🧰 Tools
🪛 LanguageTool
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...` directory is deleted for a full state reset then bootstrapping will need to be run ...(AI_HYDRA_LEO_MISSING_COMMA)
49-55: Running Localnet Section: Style Variation
The localnet instructions are clear. To keep the reader engaged, consider varying the phrasing in the guidance about cloning and checking out a specific release version.🧰 Tools
🪛 LanguageTool
[style] ~54-~54: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...release version is being used, you will need to cloneflow-goand checkout that versi...(REP_NEED_TO_VB)
61-63: Retention of Version Number Instruction
The instruction “Keep the version number used to hand” could be clarified as “Keep the version number at hand” to improve naturalness.🧰 Tools
🪛 LanguageTool
[style] ~62-~62: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...rsion number used to hand since it will need to be matched with thego.modflow-go de...(REP_NEED_TO_VB)
64-69: Bootstrap Originator Accounts Section Clarity
The section clearly explains the need for uniqueACCOUNT_NAMEvalues. Consider varying the phrasing to avoid repetition in similar contexts elsewhere in the document.🧰 Tools
🪛 LanguageTool
[style] ~66-~66: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...e or more originator accounts. You will need to set a uniqueACCOUNT_NAMEas an env v...(REP_NEED_TO_VB)
93-98: Testing and Validation Sequence: Punctuation Suggestion
Consider inserting a comma after “bootstrapped with funded originator accounts” to improve flow and readability.🧰 Tools
🪛 LanguageTool
[typographical] ~95-~95: Consider adding a comma.
Context: ...strapped with funded originator accounts then it's possible to start testing. It's ad...(IF_THEN_COMMA)
117-118: Data Directory Cleanup Instruction: Punctuation Improvement
Adding a comma in line 117 after “previous runs” can enhance readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~117-~117: Possible missing comma found.
Context: ...ba9cbdcb7aa29c86dcf ``` Deletedata/folder which stores state from previous runs a...(AI_HYDRA_LEO_MISSING_COMMA)
125-130: Start Rosetta Command Block is Clear
The instructions for starting Rosetta and the associated command are well-formatted. Consider adding a comma after “If successful” to improve flow.
146-152: Rosetta Syncing Instruction: Punctuation Suggestion
In line 148, inserting a comma after “Before continuing to test” would improve clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~148-~148: Possible missing comma found.
Context: ...-lived networks. Before continuing to test you must wait for Rosetta to confirm it...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
151-151: Hard tabs
Column: 29(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 34(MD010, no-hard-tabs)
154-163: Derived Accounts Creation: Punctuation and Style Enhancements
In line 159, add a comma after “Before proceeding” to aid readability. Additionally, consider rephrasing line 160 to avoid repetitive wording while maintaining clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~159-~159: Possible missing comma found.
Context: ...ther non Rosetta transactions. Before proceeding check${ROSETTA_ENV}.jsonto confirm ...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~160-~160: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...be stopped, and the data directory will need to be deleted before restarting it. This ...(REP_NEED_TO_VB)
198-206: Flow-go Update Guidance: Clarification Needed
The update guidance is clear; however, consider adding a comma after the introductory “In general” (line 200) for improved clarity. Verify that the specified flow-go version aligns across documentation.🧰 Tools
🪛 LanguageTool
[typographical] ~200-~200: Consider adding a comma after this introductory phrase.
Context: ...rs) ``` ## Flow-go update guidance In general Rosetta is required to be updated when ...(IN_GENERAL_COMMA)
[uncategorized] ~201-~201: Possible missing comma found.
Context: ...specificflow-goversion tag has been confirmed you can update Rosetta with it. ```ba...(AI_HYDRA_LEO_MISSING_COMMA)
207-210: Clean Localnet Environment Reminder
The instructions for bootstrapping a clean localnet environment post-compilation are clear. A comma in line 208 may enhance clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...same version dependency for the desired release you will need to bootstrap a clean `loc...(AI_HYDRA_LEO_MISSING_COMMA)
go.mod (1)
318-320: Custom Fork Replacement Directive
The replacement directive for “github.com/ipfs/boxo” using a custom fork is clearly documented with a reference to the unresolved issue (onflow/flow-go#5338). Remember to revisit this directive once the upstream issue is resolved.README.md (4)
86-88: Consistent Terminology Recommendation
Consider replacing “Sub-account” with “subaccount” (i.e. a single word) to align with common usage and improve consistency in terminology.🧰 Tools
🪛 LanguageTool
[misspelling] ~87-~87: This word is normally spelled as one.
Context: ...t originator account owned by Coinbase. Sub-account creation is handled through the Rosetta...(EN_COMPOUNDS_SUB_ACCOUNT)
210-215: Simplify Wordy Phrasing
The phrase “In order to support balance reconciliation byrosetta-cli check:data…” could be simplified to “To support balance reconciliation byrosetta-cli check:data…” for brevity and clarity.🧰 Tools
🪛 LanguageTool
[style] ~215-~215: Consider a shorter alternative to avoid wordiness.
Context: ... all accounts on-chain from genesis. * In order to support balance reconciliation by `rose...(IN_ORDER_TO_PREMIUM)
390-390: Fix List Indentation in Markdown
There is an indentation inconsistency for list items around this line. Ensure that all list items are indented uniformly (preferably 2 spaces) to adhere to Markdown linting rules and improve readability.
[style]🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
390-390: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0(MD005, list-indent)
2800-2808: Overall Documentation Structure Suggestion
The README now provides an impressively detailed walkthrough covering architecture, configuration, and end-to-end examples of using the Rosetta Construction API. However, given its length and density, consider splitting it into multiple modular documents (for example, separate files for deployment, configuration, and API usage guides). This would enhance maintainability and make it easier for users to locate specific information.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2802-2802: Link and image reference definitions should be needed
Unused link or image reference definition: "access api"(MD053, link-image-reference-definitions)
2803-2803: Link and image reference definitions should be needed
Unused link or image reference definition: "flow blockchain"(MD053, link-image-reference-definitions)
2804-2804: Link and image reference definitions should be needed
Unused link or image reference definition: "rosetta api"(MD053, link-image-reference-definitions)
2805-2805: Link and image reference definitions should be needed
Unused link or image reference definition: "flow-go"(MD053, link-image-reference-definitions)
2806-2806: Link and image reference definitions should be needed
Unused link or image reference definition: "relic build"(MD053, link-image-reference-definitions)
2807-2807: Link and image reference definitions should be needed
Unused link or image reference definition: "sporks.json"(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.summodel/model.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (30)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)Dockerfile(1 hunks)Makefile(1 hunks)README.md(8 hunks)access/access.go(1 hunks)api/network_service.go(1 hunks)cmd/corecontracts/corecontracts.go(0 hunks)cmd/genkey/genkey.go(1 hunks)cmd/tipheight/tipheight.go(0 hunks)cmd/txinfo/txinfo.go(2 hunks)config/config.go(1 hunks)go.mod(9 hunks)integration_test.py(0 hunks)localnet.json(1 hunks)mainnet-offline.json(0 hunks)model/model.proto(0 hunks)previewnet.json(0 hunks)previewnet_integration_test.py(0 hunks)rosetta_handler.py(1 hunks)script/README.md(1 hunks)script/flow.json(1 hunks)state/convert.go(1 hunks)state/process.go(1 hunks)state/state.go(0 hunks)testnet-clone.json(0 hunks)testnet-offline.json(0 hunks)testnet.json(1 hunks)testnet_integration_test.py(0 hunks)version/version.go(1 hunks)
💤 Files with no reviewable changes (11)
- model/model.proto
- cmd/corecontracts/corecontracts.go
- previewnet.json
- testnet_integration_test.py
- testnet-offline.json
- cmd/tipheight/tipheight.go
- mainnet-offline.json
- state/state.go
- integration_test.py
- previewnet_integration_test.py
- testnet-clone.json
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- access/access.go
- state/process.go
- state/convert.go
🧰 Additional context used
🪛 LanguageTool
script/README.md
[grammar] ~9-~9: Did you mean “fewer”? The noun “steps” is countable.
Context: ...testnet or mainnet there are relatively less steps since originator accounts have al...
(FEWER_LESS)
[uncategorized] ~33-~33: Possible missing comma found.
Context: ...## Configure target environment Before starting ensure that variable constants in the p...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~39-~39: Possible missing comma found.
Context: ...originator accounts. If testing a live network first time setup steps are only require...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...` directory is deleted for a full state reset then bootstrapping will need to be run ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~54-~54: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...release version is being used, you will need to clone flow-go and checkout that versi...
(REP_NEED_TO_VB)
[style] ~62-~62: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...rsion number used to hand since it will need to be matched with the go.mod flow-go de...
(REP_NEED_TO_VB)
[style] ~66-~66: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...e or more originator accounts. You will need to set a unique ACCOUNT_NAME as an env v...
(REP_NEED_TO_VB)
[typographical] ~95-~95: Consider adding a comma.
Context: ...strapped with funded originator accounts then it's possible to start testing. It's ad...
(IF_THEN_COMMA)
[style] ~102-~102: In American English, abbreviations like “etc.” require a period.
Context: ...tusing the same versions offlow-goetc 5. Test and validate Rosetta againstl...
(ETC_PERIOD)
[uncategorized] ~117-~117: Possible missing comma found.
Context: ...ba9cbdcb7aa29c86dcf ``` Delete data/ folder which stores state from previous runs a...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~131-~131: Possible missing comma found.
Context: ...ion for your target environment If successful it should log something like this b...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~148-~148: Possible missing comma found.
Context: ...-lived networks. Before continuing to test you must wait for Rosetta to confirm it...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~159-~159: Possible missing comma found.
Context: ...ther non Rosetta transactions. Before proceeding check ${ROSETTA_ENV}.json to confirm ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~160-~160: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...be stopped, and the data directory will need to be deleted before restarting it. This ...
(REP_NEED_TO_VB)
[typographical] ~200-~200: Consider adding a comma after this introductory phrase.
Context: ...rs) ``` ## Flow-go update guidance In general Rosetta is required to be updated when ...
(IN_GENERAL_COMMA)
[uncategorized] ~201-~201: Possible missing comma found.
Context: ...specific flow-go version tag has been confirmed you can update Rosetta with it. ```ba...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...same version dependency for the desired release you will need to bootstrap a clean `loc...
(AI_HYDRA_LEO_MISSING_COMMA)
README.md
[misspelling] ~87-~87: This word is normally spelled as one.
Context: ...t originator account owned by Coinbase. Sub-account creation is handled through the Rosetta...
(EN_COMPOUNDS_SUB_ACCOUNT)
[style] ~215-~215: Consider a shorter alternative to avoid wordiness.
Context: ... all accounts on-chain from genesis. * In order to support balance reconciliation by `rose...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
script/README.md
135-135: Hard tabs
Column: 29
(MD010, no-hard-tabs)
135-135: Hard tabs
Column: 34
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 29
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 34
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 29
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 34
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 29
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 34
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 29
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 34
(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 29
(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 34
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 29
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 34
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 29
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 34
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 29
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 34
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 29
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 34
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 29
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 34
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 29
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 34
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 29
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 34
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 29
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 34
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 29
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 34
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 29
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 34
(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 29
(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 34
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 29
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 34
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 29
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 35
(MD010, no-hard-tabs)
README.md
390-390: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0
(MD005, list-indent)
🔇 Additional comments (60)
rosetta_handler.py (6)
1-5: Imports look correct.
They provide the necessary libraries (JSON, subprocess, requests, click) for CLI commands and HTTP requests.
7-10: CLI group definition is well-structured.
Using “@click.group()” to define a group for subcommands is a good approach for managing multiple commands in a clean, organized manner.
20-36: Validate preprocess response fields.
The function returns whatever is given by the endpoint. Ensure usage points always check the presence and type of "options" to avoid KeyError or unexpected attribute issues.
99-110: Check result of the transaction submission.
Currently, you return the JSON but don’t check for errors. Consider verifying the transaction status code or “submitted” field to confirm success.
215-218: Registering commands is well-managed.
Commands are added to thecligroup cleanly. This approach eases future enhancements for new subcommands.
219-221: CLI entry point is straightforward.
This standard “if name == ‘main’…” block is fine for delegating to Click’s CLI.version/version.go (1)
5-6: Verify updated version strings.
Ensure the new “Flow” (0.38.0-util...) and “FlowRosetta” (0.8.9) are compatible with your codebase and any external tools that rely on version matching.cmd/genkey/genkey.go (3)
5-5: Importing the “flag” package is appropriate.
This aligns with the new CLI flag usage for CSV output.
14-15: CSV output flag is well-defined.
Providing a boolean flag allows users to easily switch output formats.
17-28: Key generation logic is correct.
You properly handle the potential error, and you generate uncompressed and compressed forms for different use cases.api/network_service.go (1)
32-32: Verify if MiddlewareVersion can be removed.Let's verify if the MiddlewareVersion field is truly unused before considering its removal.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any usage of MiddlewareVersion in the codebase rg -A 3 "MiddlewareVersion"Length of output: 290
MiddlewareVersion is only assigned and not used elsewhere.
The grep search confirms that its only appearance is in the assignment within api/network_service.go. There are no other references to this field in the codebase, so it's safe to remove the MiddlewareVersion code.
cmd/txinfo/txinfo.go (2)
19-23: LGTM! Network configuration simplified.The removal of previewnet from the networks map aligns with the broader changes across the codebase to remove previewnet support.
74-74: LGTM! Usage message updated.Usage message correctly updated to reflect the removal of previewnet option.
config/config.go (1)
211-211: LGTM! Network validation updated.The network validation has been correctly updated to only allow mainnet, testnet, localnet, or emulator networks, aligning with the removal of previewnet support.
Dockerfile (1)
12-12: Verify the impact of removing relic build tag.The build process has been simplified by removing the relic-related steps. Please confirm that this change is intentional and won't impact any cryptographic functionality that might have depended on the relic library.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any usage of relic in the codebase rg -i "relic" # Check if any Go files use relic-specific imports rg -t go "import.*relic"Length of output: 133
Relic build tag removal appears safe.
The repository search shows that aside from a mention in README.md, there are no references to the relic library in Go files or elsewhere in the code. This suggests that removal of the relic build tag from the Dockerfile is intentional and unlikely to impact functionality.
.github/workflows/ci.yml (2)
4-6: Add GO_VERSION Environment Variable
The addition of the env block with GO_VERSION set to '1.23' centralizes the Go version configuration so that it’s consistent with the go.mod upgrade.
14-19: Upgrade GitHub Actions Versions and Go Setup
Changing the checkout action to actions/checkout@v4 and updating the Go setup to actions/setup-go@v5 (with caching enabled and using the defined GO_VERSION) improves stability and brings the workflow up to date.localnet.json (4)
2-7: Configure Construction Access Nodes
Populating the "construction_access_nodes" array with a single entry (address "127.0.0.1:4001") and setting "cache" to false simplifies the localnet configuration. Verify that this accurately represents your deployment topology.
8-13: Update Contract Identifiers
The contracts object now reflects new identifiers (for flow_cold_storage_proxy, flow_fees, flow_token, and fungible_token). Please confirm these addresses match those required by the latest flow-go upgrade.
17-20: Define Originator Accounts
Adding the two originators ("1beecc6fef95b62e" and "ff8975b2fe6fb6f1") is clear. Ensure that any downstream services relying on these identifiers are updated accordingly.
24-32: Update Sporks Configuration
The spork version is updated to 7, and the entry under spork “19” now correctly references an access node at "127.0.0.1:4001". Confirm that this change is aligned with the networking upgrades in your project.testnet.json (2)
24-31: Modify Spork "51" Access Node
The address under spork "51" has been changed to "access-001.devnet51.nodes.onflow.org:9000" and the version field is now set to 7. This update should improve connectivity; verify that any consumers of this configuration can handle the new address format.
32-40: Add New Spork Entry "52"
A new spork entry "52" is added with its access node, root_block, and version details. Make sure that the introduction of this spork is coordinated with any related network or account management updates in the system.script/flow.json (2)
10-15: Expand Networks Configuration
New entries for "testing" (with address "127.0.0.1:3569") and "localnet" ("192.168.1.186:4001") in the "networks" section provide additional deployment targets and testing environments. This enhances flexibility; please verify that these endpoints are correct for your environment.
27-36: Introduce localnet-service-account
The new account "localnet-service-account" is added with a distinct key configuration. Notably, it uses "SHA2_256" for the hash algorithm instead of SHA3_256 (used elsewhere). Confirm this is intentional and compatible with your signing requirements.Makefile (7)
14-17: Ensure Consistent Build Process
The go-build target now leverages the defined compiler flags to build the server binary. Confirm that the server code is compatible with these flags and that no inadvertent behavior is introduced.
18-43: Review gen-originator-account Target
This target generates keys and creates a new account via a Go script and jq to update the flow JSON and environment JSON files. A couple of points:
• The jq command uses "$(ACCOUNT_NAME)"—ensure that ACCOUNT_NAME is defined in your environment or Makefile configuration.
• The routine appends account details to account-keys.csv; make sure concurrent invocations are handled if applicable.
44-51: Validate fund-accounts Target Loop
The fund-accounts target reads account addresses from account-keys.csv and sends funds using a cadence script. Consider adding error handling if the CSV file is missing or improperly formatted.
52-70: Ensure Robustness in create-originator-derived-account Target
This target generates keys for a derived account and extracts originator details via grep from ACCOUNT_KEYS_FILENAME based on $(ORIGINATOR_NAME). Please verify that:
• The ORIGINATOR_NAME variable is properly defined and unique in the CSV.
• Repeated grep calls are reliable and perform as expected.
71-81: Cross-Check rosetta-transfer-funds Target
This target extracts payer and recipient information from account-keys.csv to perform a fund transfer using a Python helper. Verify that the payer and recipient names are correctly set and that the CSV format remains consistent.
82-106: Enhance verify-configured-contract-addresses Target
The target uses jq to list expected contracts from the environment JSON and then verifies their deployment by inspecting flow account details. Consider:
• Emitting an error (or failing the build) if a contract is missing rather than only logging a message.
• Normalizing contract names may require additional handling if naming conventions evolve.
107-137: Review Cleanup and Ancillary Targets
The remaining targets (deps, fix-lint, lint, proto, test-reset, test-cleanup) follow standard usage. They are well organized; however, ensure that deleting files like flow.json or account-keys.csv in test-cleanup doesn’t inadvertently remove needed deployment information when switching environments.script/README.md (22)
1-2: Introduction Section: Clear Overview
The introduction clearly outlines that this directory contains Cadence scripts and transactions for Flow Rosetta.
3-4: Header Renaming Improves Clarity
Renaming the dev guide to “Rosetta Testing and Upgrade Validation Guide” sets clear expectations and enhances readability.
12-15: Prerequisites Section is Informative
The prerequisites section concisely informs the reader about the need to know Cadence when changes are made.
16-21: Flow CLI Installation Instructions are Clear
The instructions—including the one‑line installation command and version check—are clearly presented.
23-30: Python Dependency Check Block is Adequate
The steps for verifying python3 and installing the Click dependency via pip are straightforward and clear.
56-60: Git Clone Instructions are Clear
The git clone and checkout commands are correctly formatted and easy to follow.
70-72: Originator Account Creation Command Looks Good
The command for generating the originator account is succinct and well-presented.
73-81: Detailed Account Creation Steps are Informative
The bulleted list detailing key actions (key generation, account creation, JSON updates, etc.) is comprehensive and helpful.
82-88: Fund Accounts Section is Well Defined
The instructions for funding accounts using the provided make target are clear and precise.
90-91: Explanation of Funding Process is Clear
The explanation that 100 FLOW is funded for each row entry in the CSV is simple and effective.
107-112: Environment Check Instructions are Clear
The pre-testing environment check steps are well-explained and useful for troubleshooting.
113-115: Error Log Example is Informative
The provided Go error log sample effectively illustrates an issue that might occur on startup.
119-123: Build Rosetta Instructions are Clear
The build instructions using themake buildtarget are concise and straightforward.
133-145: Sample Output Code Block Formatting
The sample log output helps verify successful startup. Note that multiple hard tabs are used; replacing them with spaces may help conform to markdown lint (MD010) guidelines.A possible diff suggestion:
- INFO [cache/badger] All 0 tables opened in 0s +INFO [cache/badger] All 0 tables opened in 0s🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
135-135: Hard tabs
Column: 29(MD010, no-hard-tabs)
135-135: Hard tabs
Column: 34(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 29(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 34(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 29(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 34(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 29(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 34(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 29(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 34(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 29(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 34(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 29(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 34(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 29(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 34(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 29(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 34(MD010, no-hard-tabs)
164-166: Sub-Account Creation Command is Clear
The command for creating a derived account via the target is clear and correctly documented.
168-172: Log Emission Example is Informative
The sample logs illustrating account creation are detailed and help set the right expectations.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
171-171: Hard tabs
Column: 29(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 34(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 29(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 34(MD010, no-hard-tabs)
174-182: Account Balance Explanation is Clear
The explanation regarding the counting of account balances is precise and useful.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
174-174: Hard tabs
Column: 29(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 34(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 29(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 34(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 29(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 34(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 29(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 34(MD010, no-hard-tabs)
183-190: Transfer Funds Section is Concise
The instructions for triggering a fund transfer, including the command example, are clearly stated.
191-197: Fund Transfer Log Example is Informative
The provided log output for a fund transfer demonstrates successful tracking.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
195-195: Hard tabs
Column: 29(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 34(MD010, no-hard-tabs)
211-213: Additional Dependency Update Reminder
The note stating the need to update other dependencies, when required, is clear and appropriately placed.
214-222: Troubleshooting Guidance is Solid
The troubleshooting section effectively outlines potential misconfigurations causing indexer errors.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
221-221: Hard tabs
Column: 29(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 35(MD010, no-hard-tabs)
223-230: Verification Command is Clear
The instructions and command for verifying deployed contract addresses are straightforward and useful for operational checks.go.mod (4)
1-6: Go Version and Toolchain Update
The module now targets Go 1.23 with an updated toolchain (go1.23.4). This upgrade should help maintain compatibility with newer language features and improvements.
7-36: Primary Dependencies Updated
Key dependencies (e.g. rosetta-sdk-go, secp256k1, go-ethereum, cadence, crypto, flow-go, etc.) have been updated to newer versions. Verify that these updates are fully compatible (e.g. API changes, potential breaking changes) across the codebase and that tests pass as expected.
37-60: Indirect Dependencies Organized
The indirect dependencies block has been updated with several new versions. It’s important to ensure that these transitive updates do not introduce conflicts. Consider reviewing CI/build results after merging.
61-317: Comprehensive Indirect Dependency Management
The additional require blocks for indirect dependencies are well organized. The extensive updates across many packages indicate proactive maintenance. Verify that the updated versions are compatible with your project.README.md (2)
1-2: Improved Accessibility in the Introduction
The addition of direct hyperlinks to the Rosetta API and Flow blockchain documentation immediately gives readers quick access to essential resources, improving onboarding and navigation.
77-90: Expanded Overview Enhances Clarity
The updated overview now explains the renaming of Rosetta to Mesh and stresses that all interactions with Flow must adhere to the Rosetta standard. This comprehensive explanation improves the reader’s understanding of the integration requirements.🧰 Tools
🪛 LanguageTool
[misspelling] ~87-~87: This word is normally spelled as one.
Context: ...t originator account owned by Coinbase. Sub-account creation is handled through the Rosetta...(EN_COMPOUNDS_SUB_ACCOUNT)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Makefile (2)
44-51: Fund-Accounts Target Reads and Processes CSV Data Correctly.
The while-loop reads account addresses from account-keys.csv and trims whitespace using xargs before sending transactions. It might be beneficial to add error handling in case the CSV file is missing or malformatted.
82-106: Contract Address Verification Logic is Sound.
This target uses jq and a shell loop to iterate over expected contracts and verify deployment addresses. The approach of removing underscores and lowercasing contract names is clever. Consider enhancing error handling for cases where the flow accounts command returns unexpected results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(1 hunks)
🔇 Additional comments (13)
Makefile (13)
1-9: Environment Constants Setup Looks Good.
The environment variables (ACCOUNT_KEYS_FILENAME, FLOW_JSON, FLOW_JSON_NETWORK, FLOW_JSON_SIGNER, FLOW_CLI_FLAGS, and COMPILER_FLAGS) are clearly defined and easy to locate.
7-8: Confirming Rosetta Environment Configuration.
ROSETTA_ENV and ROSETTA_HOST_URL are set as expected. This mirrors previous suggestions and ensures consistency with the broader configuration strategy.
14-17: Go-Build Target is Structured Correctly.
The go-build target employs the defined compiler flags to build the server binary from cmd/server/server.go. Verify that the source path and flags remain in sync with your build requirements.
18-43: Robust Gen-Originator-Account Target Implementation.
This target efficiently generates keys (via go run), creates a Flow account, and updates JSON configuration files using jq. Ensure that environment variables—such as ACCOUNT_NAME—are defined externally, and consider adding additional error handling if key generation fails.
52-70: Create-Originator-Derived-Account Target is Well Structured.
This target generates new keys, extracts originator details via grep from $(ACCOUNT_KEYS_FILENAME), and invokes a Python script to create a derived account. Please verify that the required variables (such as ORIGINATOR_NAME and NEW_ACCOUNT_NAME) are set and consider additional error checking if the grep commands fail to find the expected data.
71-81: Rosetta-Transfer-Funds Target Consolidates Data Effectively.
The target extracts payer and recipient details using grep and calls a Python script to transfer funds. Ensure that environment variables like PAYER_NAME, RECIPIENT_NAME, and AMOUNT are properly defined to avoid runtime issues.
107-109: Build Target Simplifies Execution.
The build target cleanly delegates to the go-build target, ensuring consistency.
110-113: Deps Target Looks Good.
The deps target downloads the necessary Go modules. No issues detected.
114-117: Fix-Lint Target is Appropriately Configured.
Running golangci-lint with the --fix flag automatically addresses linting issues.
118-122: Lint Target is Clean and Efficient.
Tidying modules and running static analysis via staticcheck is a solid approach to maintain code quality.
123-128: Proto Target Facilitates Protobuf Generation.
This target generates model/model.pb.go using protoc. Just ensure that any changes to model/model.proto are revalidated against this command.
129-132: Test-Reset Target Clears Test Artifacts Effectively.
The test-reset target removes the data directory, ensuring prior test states don’t interfere with new ones.
133-137: Test-Cleanup Target Ensures a Clean Test Environment.
By deleting flow.json, account-keys.csv, and the flow-go directory, this target effectively cleans up generated files.
There was a problem hiding this comment.
I made the decision to port a number of key functions that were previously mastered in $ENV_integration_test.py into the Makefile. Overall the use of the python script made things more complicated than needed, since anyway it was calling out to go or other command line tools for the most part.
The only functions which I did not migrate were those which implement the Rosetta API invocation which remain grandfathered in the python script
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
localnet.json (2)
8-13: Confirm Contract Identifiers and Address Potential Static Analysis False Positives.
Thecontractssection has been updated with new hexadecimal identifiers:
"flow_cold_storage_proxy": "0000000000000000""flow_fees": "e5a8b7f23e8b548f""flow_token": "0ae53cb6e3f42a79""fungible_token": "ee82856bf20e2aa6"These appear correct for the updated configuration. Note that one of these values triggered a static analysis warning (a generic API key alert); if these values are public contract identifiers, document that they are not sensitive to avoid future confusion.
🧰 Tools
🪛 Gitleaks (8.21.2)
11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
17-18: Assess "originators" Array Initialization.
The"originators": []field remains empty. If this is intended for later population (or for specific test scenarios), consider adding an inline comment to clarify its purpose.script/README.md (6)
1-7: Introduction Clarity & Punctuation – Nitpick.
The introductory section clearly explains the purpose of the directory. Consider reviewing the punctuation (e.g., inserting commas where natural pauses occur) to further enhance readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Possible missing comma found.
Context: ...e, internal components from the flow-go repo subjecting Rosetta to upstream breaking...(AI_HYDRA_LEO_MISSING_COMMA)
43-46: Punctuation Suggestion in CLI Instructions – Nitpick.
In the explanation following the code block for starting the emulator, consider adding a comma after “--persistflag” for improved clarity. For example:-Be aware that when using the `--persist` flag the emulator will preserve state between starts locally in `flowdb/` directory. +Be aware that when using the `--persist` flag, the emulator will preserve state between starts locally in the `flowdb/` directory.🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: Possible missing comma found.
Context: ...e aware that when using the--persistflag the emulator will preserve state betwee...(AI_HYDRA_LEO_MISSING_COMMA)
95-97: Punctuation in Testing Instructions – Nitpick.
In the sentence starting “Once the environment is bootstrapped with funded originator accounts then it's possible to start testing…”, inserting a comma after “funded originator accounts” can enhance readability.🧰 Tools
🪛 LanguageTool
[typographical] ~95-~95: Consider adding a comma.
Context: ...strapped with funded originator accounts then it's possible to start testing. It's ad...(IF_THEN_COMMA)
159-160: Clarify Account Status Check – Nitpick.
When instructing users to check${ROSETTA_ENV}.jsonfor listed originators, consider adding a comma for clarity after “first run”. For example:-...once the originator accounts are created, Rosetta will need to be stopped, and the data directory will need to be deleted before restarting it. +...once the originator accounts are created, Rosetta will need to be stopped, and the data directory will need to be deleted before restarting it.Even if the diff is trivial, ensuring clear punctuation in such important instructions is helpful.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~159-~159: Possible missing comma found.
Context: ...ther non Rosetta transactions. Before proceeding check${ROSETTA_ENV}.jsonto confirm ...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~160-~160: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...be stopped, and the data directory will need to be deleted before restarting it. This ...(REP_NEED_TO_VB)
135-143: Markdown Hard Tabs – Nitpick.
Multiple static analysis warnings indicate the presence of hard tabs in several code blocks. To adhere to markdownlint’s MD010 guideline, consider replacing tabs with spaces for consistency.Also applies to: 151-152, 171-176, 221-221
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
135-135: Hard tabs
Column: 29(MD010, no-hard-tabs)
135-135: Hard tabs
Column: 34(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 29(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 34(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 29(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 34(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 29(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 34(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 29(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 34(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 29(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 34(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 29(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 34(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 29(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 34(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 29(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 34(MD010, no-hard-tabs)
208-210: Punctuation in Flow-go Guidance – Nitpick.
In the sentence “Once Rosetta compiles using the same version dependency for the desired release you will need to bootstrap a clean localnet environment…”, consider adding a comma after “for the desired release” to improve readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...same version dependency for the desired release you will need to bootstrap a clean `loc...(AI_HYDRA_LEO_MISSING_COMMA)
README.md (6)
79-89: Enhanced Overview & Language Improvements:
The expanded overview now explains Rosetta’s evolution (now “Mesh”) and its integration requirements more thoroughly. A few suggestions:
- Consider inserting a comma after “Rosetta” in the sentence beginning around line 85 (e.g., “...historical design assumptions made by Rosetta, certain additional requirements must…”) to improve readability.
- In line 87, review whether “sub-account” should be styled as “subaccount” in keeping with your project’s naming conventions.
- The phrase “requires a
getBalanceequivalent to function” might be improved by rephrasing it as “requires a function equivalent togetBalance” to eliminate redundancy.🧰 Tools
🪛 LanguageTool
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...c historical design assumptions made by Rosetta certain additional requirements must b...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~87-~87: This word is normally spelled as one.
Context: ...t originator account owned by Coinbase. Sub-account creation is handled through the Rosetta...(EN_COMPOUNDS_SUB_ACCOUNT)
193-196: Config Files Clarification:
The updated “### Config files” section explains that the provided configuration files are primarily for reference and testing. For improved clarity, consider adding a comma after “operationally” (in line 194) so that it reads:“Since Coinbase runs Rosetta operationally, the contents of …”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~193-~193: Possible missing comma found.
Context: ...d testing. Since Coinbase runs Rosetta operationally the contents oftestnet.jsonand `mai...(AI_HYDRA_LEO_MISSING_COMMA)
205-208: Balance Reconciliation Documentation:
The documentation regarding support for balance reconciliation (usingrosetta-cli check:data) is much clearer now. Consider rephrasing “In order to support balance reconciliation byrosetta-cli check:data” to “To support balance reconciliation byrosetta-cli check:data,” which is more concise.🧰 Tools
🪛 LanguageTool
[uncategorized] ~206-~206: Possible missing comma found.
Context: ...sponding value movements on-chain, from genesis with no exceptions. However, incompatib...(AI_HYDRA_LEO_MISSING_COMMA)
227-227: Punctuation Suggestion:
Within the section describing network spork behavior (around line 227), adding a comma after “Rosetta” (before “functionality”) could help break up the sentence and improve flow.🧰 Tools
🪛 LanguageTool
[uncategorized] ~227-~227: Possible missing comma found.
Context: ...requently release breaking changes to a network requiring downtime and a halt in Rosett...(AI_HYDRA_LEO_MISSING_COMMA)
244-246: Grammatical Correction:
In the note regarding contract addresses, the sentence at line 246 currently starts with “Its important…”. Replace “Its” with “It’s” to correct the grammar.🧰 Tools
🪛 LanguageTool
[uncategorized] ~246-~246: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...source for spork versioning on Flow. Its important that changes to spork version...(AI_HYDRA_LEO_CPT_ITS_ITIS)
390-390: Markdown Styling:
A static analysis hint flagged an inconsistency with list-item indentation around line 390. Please ensure that all list items use consistent indentation (typically 2 spaces) for better readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
390-390: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0(MD005, list-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(8 hunks)localnet.json(1 hunks)rosetta_handler.py(1 hunks)script/README.md(1 hunks)script/flow.json(1 hunks)version/version.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- version/version.go
- rosetta_handler.py
🧰 Additional context used
🪛 Biome (1.9.4)
script/flow.json
[error] 38-38: expected } but instead the file ends
the file ends here
(parse)
🪛 LanguageTool
script/README.md
[uncategorized] ~6-~6: Possible missing comma found.
Context: ...e, internal components from the flow-go repo subjecting Rosetta to upstream breaking...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~9-~9: Did you mean “fewer”? The noun “steps” is countable.
Context: ...testnet or mainnet there are relatively less steps since originator accounts have al...
(FEWER_LESS)
[uncategorized] ~39-~39: Possible missing comma found.
Context: ...originator accounts. If testing a live network first time setup steps are only require...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~46-~46: Possible missing comma found.
Context: ...e aware that when using the --persist flag the emulator will preserve state betwee...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~54-~54: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...release version is being used, you will need to clone flow-go and checkout that versi...
(REP_NEED_TO_VB)
[style] ~62-~62: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...rsion number used to hand since it will need to be matched with the go.mod flow-go de...
(REP_NEED_TO_VB)
[style] ~66-~66: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...e or more originator accounts. You will need to set a unique ACCOUNT_NAME as an env v...
(REP_NEED_TO_VB)
[typographical] ~95-~95: Consider adding a comma.
Context: ...strapped with funded originator accounts then it's possible to start testing. It's ad...
(IF_THEN_COMMA)
[style] ~102-~102: In American English, abbreviations like “etc.” require a period.
Context: ...tusing the same versions offlow-goetc 5. Test and validate Rosetta againstl...
(ETC_PERIOD)
[uncategorized] ~117-~117: Possible missing comma found.
Context: ...ba9cbdcb7aa29c86dcf ``` Delete data/ folder which stores state from previous runs a...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~131-~131: Possible missing comma found.
Context: ...ion for your target environment If successful it should log something like this b...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~148-~148: Possible missing comma found.
Context: ...-lived networks. Before continuing to test you must wait for Rosetta to confirm it...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~159-~159: Possible missing comma found.
Context: ...ther non Rosetta transactions. Before proceeding check ${ROSETTA_ENV}.json to confirm ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~160-~160: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...be stopped, and the data directory will need to be deleted before restarting it. This ...
(REP_NEED_TO_VB)
[uncategorized] ~191-~191: Possible missing comma found.
Context: ... The indexer will emit a log similar to this which is indicative that the fund trans...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~200-~200: Consider adding a comma after this introductory phrase.
Context: ...rs) ``` ## Flow-go update guidance In general Rosetta is required to be updated when ...
(IN_GENERAL_COMMA)
[uncategorized] ~201-~201: Possible missing comma found.
Context: ...specific flow-go version tag has been confirmed you can update Rosetta with it. ```ba...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...same version dependency for the desired release you will need to bootstrap a clean `loc...
(AI_HYDRA_LEO_MISSING_COMMA)
README.md
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...c historical design assumptions made by Rosetta certain additional requirements must b...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~87-~87: This word is normally spelled as one.
Context: ...t originator account owned by Coinbase. Sub-account creation is handled through the Rosetta...
(EN_COMPOUNDS_SUB_ACCOUNT)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...d requires a getBalance equivalent to function. * Uses the Access API servers to g...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~193-~193: Possible missing comma found.
Context: ...d testing. Since Coinbase runs Rosetta operationally the contents of testnet.json and `mai...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~206-~206: Possible missing comma found.
Context: ...sponding value movements on-chain, from genesis with no exceptions. However, incompatib...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~215-~215: Consider a shorter alternative to avoid wordiness.
Context: ... all accounts on-chain from genesis. * In order to support balance reconciliation by `rose...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~227-~227: Possible missing comma found.
Context: ...requently release breaking changes to a network requiring downtime and a halt in Rosett...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~246-~246: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...source for spork versioning on Flow. Its important that changes to spork version...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
🪛 markdownlint-cli2 (0.17.2)
script/README.md
135-135: Hard tabs
Column: 29
(MD010, no-hard-tabs)
135-135: Hard tabs
Column: 34
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 29
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 34
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 29
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 34
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 29
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 34
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 29
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 34
(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 29
(MD010, no-hard-tabs)
140-140: Hard tabs
Column: 34
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 29
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 34
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 29
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 34
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 29
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 34
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 29
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 34
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 29
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 34
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 29
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 34
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 29
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 34
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 29
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 34
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 29
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 34
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 29
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 34
(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 29
(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 34
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 29
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 34
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 29
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 35
(MD010, no-hard-tabs)
README.md
390-390: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0
(MD005, list-indent)
🪛 Gitleaks (8.21.2)
localnet.json
11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (13)
localnet.json (4)
2-7: Validate "construction_access_nodes" configuration.
The configuration now specifies a single local access node with"address": "127.0.0.1:4001". This is appropriate for a local testing environment. Ensure that this setting aligns with the requirements for the latest flow-go upgrade (v0.38.0-rc.8).
14-16: Review Data Directory and Network Settings.
The additions of"data_dir": "./data","disable_consensus_follower": true, and"network": "localnet"are clear and cogent. They establish predictable paths and testing parameters for local development.
19-21: Verify Port and Tolerance Values.
The configuration for"port": 8080,"spork_seal_tolerance": 10000, and"spork_synced_tolerance": 10is straightforward. Confirm that these numerical values meet the operational and performance needs of your refactored tooling and network simulation.
22-32: Review Updated "sporks" Configuration.
Thesporkssection now includes an entry (key"19") with updated settings:
- The
"access_nodes"list repeats"127.0.0.1:4001", consistent with the construction access node."root_block": 0and"version": 7have been set, reflecting the intended update.Double-check that the version bump and node configuration correctly match the overall network setup and that any downstream tooling is updated accordingly.
script/flow.json (3)
9-14: New Network Configurations – Approved.
The addition of the new"testing": "127.0.0.1:3569"and"localnet": "127.0.0.1:4001"entries under"networks"aligns with the updated testing requirements. Note that a previous review comment already suggested adding the"localnet"entry, so this is tagged as a duplicate.
16-26: New Account Entry Verification.
A new account,"localnet-service-account", has been added under"accounts"with a detailed key structure. Please verify that the difference in the"hashAlgorithm"(using"SHA2_256"here versus"SHA3_256"for"emulator-account") is intentional and complies with security requirements.Also applies to: 27-35
36-38: JSON Structure Verification – Check Closing Brackets.
The static analysis tool reported an error indicating an expected}near the file’s end. Please double-check that every object (especially within the"accounts"block) is properly closed. If a closing brace is indeed missing, add it to ensure the JSON is syntactically valid. For example, if the"localnet-service-account"object isn’t properly closed, apply a diff like:- } - }, + } + },🧰 Tools
🪛 Biome (1.9.4)
[error] 38-38: expected
}but instead the file endsthe file ends here
(parse)
script/README.md (2)
8-10: Countable Noun Correction – Duplicate Comment.
The phrase “relatively less steps” should be revised to “relatively fewer steps” since “steps” is a countable noun. A suggested diff:-testnet or mainnet there are relatively less steps since +testnet or mainnet there are relatively fewer steps since🧰 Tools
🪛 LanguageTool
[grammar] ~9-~9: Did you mean “fewer”? The noun “steps” is countable.
Context: ...testnet or mainnet there are relatively less steps since originator accounts have al...(FEWER_LESS)
56-60: Flow-go Version Consistency – Verify Version Usage.
There is an inconsistency in the flow-go version instructions. The localnet setup code block instructs users to check out tagv0.37.26, yet later in the “Flow-go update guidance” section the command usesv0.38.0(and the PR description mentionsv0.38.0-rc.8). Please verify and update these instructions so they consistently guide users to the intended release version.README.md (4)
1-1: Introduction Clarity:
The updated introductory line clearly states the repository’s purpose and provides updated hyperlinks to both the Rosetta API and Flow blockchain documentation.
91-92: Section Title Update:
The new “### Design” header effectively segments the documentation and enhances the overall structure.
197-202: FlowColdStorageProxy Contract Explanation:
The revised section clearly explains that theFlowColdStorageProxycontract is not currently deployed, along with the rationale behind its development. This additional context is very helpful.
566-568: Testing Documentation Update:
The updated “Upgrading, testing and validating Rosetta updates” section now properly references thescript/README.mdfile with an informative call-to-action. This additional documentation context is very useful for users.
peterargue
left a comment
peterargue
left a comment
There was a problem hiding this comment.
looks good. added a couple small comments but nothing that should block this going out
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
go.mod (2)
82-335: Extensive Update of Extended Indirect Dependencies (Second Block)
An extensive list of dependencies has been updated in this section. Many important libraries—such as various AWS SDK modules, OpenTelemetry instrumentation packages, and several libp2p-related components—have had their versions bumped. For instance:
• Updates togo.opentelemetry.io/contrib/instrumentation/...andgo.uber.org/*packages
• Numerous changes to GitHub-hosted modules including those for protobuf, Google API, and modules spanning cryptography and networking.Because these updates impact a broad range of functionality, please ensure that you perform thorough integration testing and consider documenting any notable behavioral changes from upstream releases.
337-339: Custom Fork Replacement Directive forgithub.com/ipfs/boxo
The replacement directivereplace github.com/ipfs/boxo => github.com/onflow/boxo v0.0.0-20240201202436-f2477b92f483is a temporary measure pending resolution of this issue. This is acceptable for now, but please ensure you track the upstream issue and update or remove the fork replacement once the fix is officially released.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
go.mod (3)
3-5: Upgrade Go Version and Toolchain Declaration
The file now specifiesgo 1.23andtoolchain go1.23.4, which aligns with the latest language improvements. Please verify that all modules and tools in the project are compatible with Go 1.23.
8-36: Primary Dependency Version Updates
Numerous key dependencies have been upgraded in this block (for example:
•github.com/coinbase/rosetta-sdk-goupdated to v0.8.9
•github.com/decred/dcrd/dcrec/secp256k1/v4updated to v4.3.0
•github.com/ethereum/go-ethereumupdated to v1.14.12
•github.com/libp2p/go-libp2pupdated to v0.38.2
•github.com/onflow/cadenceupdated to v1.3.1
•github.com/onflow/cryptoupdated to v0.25.2
•github.com/onflow/flow-goupdated to v0.38.1 (note the previous feedback mentioned v0.38.0 )
•github.com/onflow/flow/protobuf/go/flowupdated to v0.4.7
•github.com/stretchr/testifyupdated to v1.10.0 and several OpenTelemetry dependencies upgraded.Please confirm that none of these version bumps introduce breaking changes and that they have been tested in your integration environment.
39-79: Indirect Dependencies (First Block) Bulk Update
This block updates several indirect dependencies (e.g.,github.com/Jorropo/jsync,github.com/cockroachdb/fifo,github.com/crate-crypto/go-ipa, among others). Given the volume of changes, it is advisable to run a dependency audit (or security scanning tool) to ensure no new vulnerabilities or compatibility issues have been introduced.
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
3 participants
Updates for latest flow-go upgrade ( v0.38.0-rc.8). Also includes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores