Update cadence/flow-go/flow-go-sdk/flow-emulator dependencies
#738
Conversation
m-Peter
requested review from
janezpodhostnik,
peterargue and
zhangchiqing
as code owners
WalkthroughThe pull request introduces updates to the project's dependencies and modifications in the bootstrap package. The Go version has been upgraded from 1.22 to 1.23, along with various dependencies updated to their latest versions. In the bootstrap package, the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(2 hunks)Dockerfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: Lint
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
19-19: Verify Go version compatibility with dependencies.Since you're updating Flow-related dependencies, let's verify their Go version requirements.
Also applies to: 36-36
✅ Verification successful
Go version 1.23.x is compatible with all dependencies ✅
The Go version specified in CI matches the version requirements in both go.mod files and is compatible with all Flow-related dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Go version requirements in go.mod files of Flow dependencies # Check go.mod files for Go version echo "Checking Go version in go.mod files..." rg -A 5 "^go \d+\.\d+" # Check if dependencies specify Go version constraints echo -e "\nChecking version constraints in go.mod..." rg "require\s+.*flow-" -A 10Length of output: 619
Script:
#!/bin/bash # Search for all Flow-related dependencies and their versions echo "Searching for Flow dependencies in go.mod files..." rg "github.com/onflow" go.mod tests/go.mod # Get the content of go.mod files for manual inspection echo -e "\nContent of go.mod files:" echo "=== go.mod ===" cat go.mod echo -e "\n=== tests/go.mod ===" cat tests/go.modLength of output: 26609
| @@ -364,10 +363,6 @@ func (b *Bootstrap) StartMetricsServer(ctx context.Context) error { | |||
| b.logger.Info().Msg("bootstrap starting metrics server") | |||
|
|
|||
| b.metrics = flowMetrics.NewServer(b.logger, uint(b.config.MetricsPort)) | |||
| err := util.WaitClosed(ctx, b.metrics.Ready()) | |||
There was a problem hiding this comment.
looks like metrics server is a component. why remove these?
There was a problem hiding this comment.
That's a good question! So the metrics server became a component in this commit: onflow/flow-go@4c8ac17.
Which means that with:
err := util.WaitClosed(ctx, b.metrics.Ready())The entire test suite would fail, due to indefinite blocking. The same goes for the shutdown of the metrics server:
<-b.metrics.Done()I had to remove the above line.
The flow-go commit I linked above, was intended to be used from this PR: #682. I am unsure as to how to proceed here..
There was a problem hiding this comment.
I'll take a look today.
There was a problem hiding this comment.
This should do it I think: #741
There was a problem hiding this comment.
This should do it I think: #741
@janezpodhostnik That piece of code works like a charm 💯 Could you prepare that PR and merge on this one?
m-Peter
force-pushed
the
mpeter/update-dependencies
branch
2 times, most recently
from
54b261f to
75031c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bootstrap/bootstrap.go (1)
371-392: LGTM! Consider enhancing error handling in the monitoring goroutine.The new metrics server startup implementation correctly handles the component lifecycle. However, the panic in the monitoring goroutine could be replaced with a more graceful shutdown mechanism.
Consider this alternative error handling approach:
go func() { err := <-errCh if err != nil { b.logger.Err(err).Msg("error in metrics server") - panic(err) + // Signal shutdown to the main context + cancel() } }()This would require passing a cancel function to StartMetricsServer:
func (b *Bootstrap) StartMetricsServer(ctx context.Context, cancel context.CancelFunc) error {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.github/workflows/ci.yml(2 hunks)Dockerfile(1 hunks)api/debug.go(1 hunks)bootstrap/bootstrap.go(2 hunks)go.mod(5 hunks)services/evm/executor.go(0 hunks)tests/go.mod(5 hunks)tests/web3js/debug_traces_test.js(2 hunks)
💤 Files with no reviewable changes (1)
- services/evm/executor.go
🚧 Files skipped from review as they are similar to previous changes (4)
- Dockerfile
- .github/workflows/ci.yml
- tests/web3js/debug_traces_test.js
- go.mod
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint
- GitHub Check: Test
🔇 Additional comments (23)
bootstrap/bootstrap.go (2)
17-17: LGTM!The import of
irrecoverablepackage is appropriate for the new metrics server lifecycle management.
401-401: LGTM!The addition of the warning log message improves observability during the metrics server shutdown process.
tests/go.mod (20)
3-3: Update Go Version Directive
The Go version is now set to 1.23, which aligns this module with the updated Dockerfile and CI configuration. Please ensure all local builds and tests run successfully with this new version.
7-7: Upgrade onflow/cadence Dependency
Thegithub.com/onflow/cadencedependency has been updated to v1.3.1. This upgrade should bring in the latest features and fixes. Double-check that any changes in its API are compatible with the rest of the integration tests.
9-9: Upgrade flow-emulator Dependency
Thegithub.com/onflow/flow-emulatordependency is now at v1.1.1-0.20250128000351-085706c390e1. Confirm that the emulator’s behavior remains consistent with your integration expectations.
11-11: Upgrade flow-go Dependency
Thegithub.com/onflow/flow-godependency has been updated to v0.38.0-util.0.20250203085701-72e6e6235fa5. This should ensure compatibility with the latest protocol improvements; please verify that no breaking changes affect your use cases.
12-12: Upgrade flow-go-sdk Dependency
Thegithub.com/onflow/flow-go-sdkis now at v1.3.1. Ensure that any API changes are accounted for across the codebase and that tests confirming SDK behavior have been updated accordingly.
15-15: Upgrade Testify Dependency
Thegithub.com/stretchr/testifylibrary has been upgraded to v1.10.0. This minor bump should offer improvements without breaking tests; please run all tests to confirm that assertions and mock functionalities behave as expected.
19-19: Upgrade Google Compute Metadata Client
The dependencycloud.google.com/go/compute/metadatahas been updated to v0.5.0. Verify that any code interacting with Google Cloud metadata behaves correctly with this newer version.
91-91: Update googleapis/gax-go Dependency
Thegithub.com/googleapis/gax-go/v2dependency is now at v2.12.2. Confirm that this update does not disrupt any gRPC-related operations or client interactions that depend on GAX for retries and error handling.
151-151: Upgrade onflow/atree Dependency
Thegithub.com/onflow/atreedependency has been updated to v0.9.0. Ensure that any tree-related operations or integrity checks relying on this module remain fully functional after the upgrade.
152-152: Add Bridged-USDC Contracts Dependency
A new dependency,github.com/onflow/bridged-usdc/lib/go/contractsat v1.0.0, has been added. Please verify that this new package is correctly integrated and that its usage is well documented across the codebase.
206-206: Upgrade zeebo/blake3 Dependency
Thegithub.com/zeebo/blake3package is now at v0.2.4, which may include important bug fixes or performance improvements for hashing. Ensure that any cryptographic functionalities using Blake3 are retested for expected behavior.
208-208: Upgrade opentelemetry otelgrpc Instrumentation
Thego.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcmodule has been updated to v0.49.0. Please confirm that gRPC tracing and metrics collection are working correctly with the new version.
209-209: Upgrade opentelemetry otelhttp Instrumentation
Thego.opentelemetry.io/contrib/instrumentation/net/http/otelhttpdependency is also updated to v0.49.0, which should complement the otelgrpc upgrade. Verify that HTTP instrumentation and tracing are performing as expected.
223-223: Upgrade golang.org/x/net Dependency
The modulegolang.org/x/nethas been upgraded to v0.26.0. This update may enhance networking functions; ensure that any custom network logic or integrations are not adversely affected.
224-224: Upgrade golang.org/x/oauth2 Dependency
Thegolang.org/x/oauth2package is now at v0.18.0. Confirm that all OAuth interactions, token retrieval, and refresh logic remain compatible after this version change.
232-232: Upgrade google.golang.org/api Dependency
Thegoogle.golang.org/apidependency has been updated to v0.169.0. Verify that all client interactions with Google APIs continue to function as intended, particularly any authentication or service discovery flows.
235-235: Upgrade Genproto for Google APIs
Thegoogle.golang.org/genproto/googleapis/apimodule is now at v0.0.0-20240814211410-ddb44dafa142. Please check that any generated code or proto-based integrations are updated and compile correctly.
236-236: Upgrade Genproto for Google RPCs
Thegoogle.golang.org/genproto/googleapis/rpcdependency has been updated to v0.0.0-20240814211410-ddb44dafa142. Validate that this change does not introduce issues in gRPC serialization or remote procedure call implementations.
237-237: Upgrade gRPC Dependency
Thegoogle.golang.org/grpclibrary has been updated to v1.64.1. This upgrade should provide performance improvements and bug fixes; ensure that any gRPC communications or service endpoints remain stable.
238-238: Upgrade Protocol Buffers Dependency
Thegoogle.golang.org/protobufmodule is now at v1.34.2. Confirm that all proto definitions and any generated code continue to work seamlessly with this updated version.api/debug.go (1)
241-248: Verify the intentional discarding of DryCall result.The
DryCallresult is now discarded using the blank identifier_. This change aligns with the removal of log processing in the broader codebase, but let's verify that this is intentional and doesn't impact error reporting or debugging capabilities.Run the following script to check for any other usages of
DryCallresults in the codebase:✅ Verification successful
Intentional Discarding of DryCall Result Confirmed
The DryCall result is intentionally discarded in
api/debug.go(line 241) to align with the removal of log processing in this context. Notably, other parts of the codebase—such as inservices/requester/requester.go—continue to capture the DryCall result, confirming that the change is specific to the Debug API’s needs. This approach is consistent with broader changes in log handling.
- In
api/debug.go, the DryCall result is ignored since its logs are no longer processed.- In
services/requester/requester.go, the DryCall return is captured for further usage, indicating context-specific handling.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how DryCall results are used across the codebase # Search for DryCall method usage patterns ast-grep --pattern 'view.DryCall($$$)' # Search for any references to DryCall logs rg -A 5 'DryCall.*Logs'Length of output: 66
Script:
#!/bin/bash # Description: Search for all occurrences of DryCall usage and its definition across the codebase echo "Searching for DryCall method usages and definitions:" rg -n "DryCall(" .Length of output: 202
Script:
#!/bin/bash # Description: Corrected search for all occurrences of DryCall usage across the codebase using proper escaping. echo "Searching for DryCall usages in the codebase:" rg -n 'DryCall\(' .Length of output: 233
Co-authored-by: Peter Argue <[email protected]>
m-Peter
force-pushed
the
mpeter/update-dependencies
branch
from
75031c7 to
b0ee1d1
Compare
|
@turbolent I have updated this PR to use the latest tagged release of |
Co-authored-by: Peter Argue <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bootstrap/bootstrap.go (1)
402-403: Move warning log before server shutdown.The warning log message should be placed before the server shutdown to ensure it's logged even if the shutdown hangs.
Apply this diff to fix the log message placement:
+ b.logger.Warn().Msg("shutting down metrics server") <-b.metrics.Done() - b.logger.Warn().Msg("shutting down metrics server")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bootstrap/bootstrap.go(3 hunks)
🧰 Additional context used
🪛 GitHub Check: Test
bootstrap/bootstrap.go
[failure] 377-377:
undefined: util
🪛 golangci-lint (1.62.2)
bootstrap/bootstrap.go
377-377: undefined: util
(typecheck)
🪛 GitHub Actions: CI
bootstrap/bootstrap.go
[error] 377-377: undefined: util
🔇 Additional comments (1)
bootstrap/bootstrap.go (1)
375-394: LGTM! Robust error handling implementation.The error handling implementation is well-designed:
- Uses
irrecoverable.WithSignalerfor proper context management- Monitors startup errors through a dedicated error channel
- Handles errors asynchronously in a goroutine
🧰 Tools
🪛 GitHub Check: Test
[failure] 377-377:
undefined: util🪛 golangci-lint (1.62.2)
377-377: undefined: util
(typecheck)
🪛 GitHub Actions: CI
[error] 377-377: undefined: util
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bootstrap/bootstrap.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (2)
bootstrap/bootstrap.go (2)
17-17: LGTM: Import of irrecoverable package.The addition of the
irrecoverablepackage is appropriate for handling component lifecycle management.
399-405: LGTM: Metrics server shutdown logic.The shutdown logic is correct and includes appropriate logging.
Description
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
Dependency Updates
github.com/onflow/bridged-usdc/lib/go/contractsat version 1.0.0Infrastructure
Bug Fixes