Standalone storage with dynamic contract#251
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR refactors the Aurora engine refiner to support contract versioning and dynamic loading. Key changes include: switching aurora-engine dependencies to a feature branch, replacing async Arc<RwLock> with direct Storage ownership using .share() for task distribution, removing the BatchIO adapter, introducing contract fetching from GitHub releases with versioning infrastructure, adding a runner module for RPC-based version tracking, refactoring transaction processing to remove raw_input fields, and introducing TxKind for transaction classification. Configuration and CLI updates support contract source specification. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/Main
participant Engine as Engine Context
participant Storage as Storage
participant RPC as NEAR RPC
participant GitHub as GitHub API
CLI->>Engine: Initialize with storage path
Note over CLI,Engine: Contract Version Fetch & Apply
CLI->>RPC: runner::version::get(height)
RPC-->>CLI: version string
CLI->>Storage: storage_ext::apply_contract(height, version)
Storage->>Storage: get_contract by height/pos
alt Contract found in storage
Storage-->>CLI: existing contract code
else Contract not found
Storage->>Storage: load_from_file(version)
Storage-->>CLI: code from disk
end
Note over CLI,GitHub: Contract Source Fetch (if configured)
alt contract_source provided
CLI->>GitHub: fetch_contract::all()
GitHub->>GitHub: fetch_releases(repo)
GitHub-->>CLI: release list
loop For each release
CLI->>GitHub: Download aurora-compat.wasm
GitHub-->>CLI: contract binary
CLI->>Storage: Store by version
end
end
Note over CLI,Storage: Socket Server Setup
CLI->>Engine: context.storage.share()
Engine-->>CLI: Shared storage reference
CLI->>Storage: start_socket_server(storage, contract_source)
Note over Storage: Runtime - Message Processing
Storage->>Storage: handle_msg with contract_source context
Storage->>Storage: route to gas/trace handlers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
vlad9486
force-pushed
the
feat/standalone-storage-with-dynamic-contract
branch
from
2ca9aa3 to
c974a6a
Compare
vlad9486
force-pushed
the
feat/standalone-storage-with-dynamic-contract
branch
3 times, most recently
from
c269a28 to
a97c073
Compare
vlad9486
force-pushed
the
feat/standalone-storage-with-dynamic-contract
branch
from
a97c073 to
2552489
Compare
| }; | ||
| use thiserror::Error; | ||
|
|
||
| struct Nop; |
There was a problem hiding this comment.
| struct Nop; | |
| struct Noop; |
Signed-off-by: Vladyslav Melnyk <vladyslav.melnyk@aurora.dev>
vlad9486
force-pushed
the
feat/standalone-storage-with-dynamic-contract
branch
from
21c230c to
6d69d32
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (9)
refiner-lib/src/storage.rs (1)
21-21: Consider propagating errors instead of panicking.Using
.unwrap()onStorage::open_ensure_account_idwill panic if the storage cannot be opened. Since this is initialization code, consider whether returning aResultwould provide better error handling upstream.- let mut storage = Storage::open_ensure_account_id(&storage_path, account_id).unwrap(); + let mut storage = Storage::open_ensure_account_id(&storage_path, account_id) + .map_err(|e| tracing::error!("Failed to open storage: {e}")) + .expect("Storage initialization failed");This at least logs the error before panicking, or consider changing
init_storageto returnResult<(), Error>.refiner-types/src/source_config.rs (1)
1-7: Add documentation and usage examples.The
ContractSourceenum lacks documentation explaining the expected format ofbase_urlandrepofields.Add documentation:
+/// Configuration for fetching contract code from an external source. #[derive(Deserialize, Clone, Debug, PartialEq, Eq)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum ContractSource { + /// Fetch contract from a GitHub release. + /// + /// # Example + /// + /// ```json + /// { + /// "kind": "github", + /// "base_url": "https://github.com", + /// "repo": "aurora-is-near/aurora-engine" + /// } + /// ``` Github { + /// Base URL of the GitHub instance (typically "https://github.com"). base_url: String, + /// Repository in "owner/repo" format. repo: String, }, }refiner-app/src/main.rs (1)
108-116: Extract hardcoded height to a named constant and address the TODO.The height
134229098is hardcoded twice and the TODO indicates this needs proper resolution. Consider extracting to a named constant and implementing proper height retrieval logic.+const DEFAULT_CONTRACT_HEIGHT: u64 = 134229098; + // TODO(vlad): get correct height - let version = runner::version::get(height.unwrap_or(134229098), true).await?; + let version = runner::version::get(height.unwrap_or(DEFAULT_CONTRACT_HEIGHT), true).await?; storage_ext::apply_contract( &mut ctx.storage, - height.unwrap_or(134229098), + height.unwrap_or(DEFAULT_CONTRACT_HEIGHT), 0, Some(version.as_str().trim_end()), args.contract_path.map(PathBuf::from), )?;engine/src/tracing/lib.rs (1)
49-50: Replaceunwrap()withexpect()or proper error handling.Using
unwrap()directly could cause a panic ifcall_traceris unexpectedlyNone. While settingtrace_kind = Some(TraceKind::CallFrame)should guarantee the tracer is populated, a defensive approach would prevent obscure panics.- let tracer = outcome.call_tracer.take().unwrap(); + let tracer = outcome + .call_tracer + .take() + .expect("call_tracer must be present when trace_kind is set");refiner-lib/src/kind.rs (1)
58-60: Consider using a more informative error type.Using
()as the error type discards information about which string failed to parse, making debugging harder. A custom error type orStringwould preserve the invalid input.impl FromStr for TxKind { - type Err = (); + type Err = String; fn from_str(s: &str) -> Result<Self, Self::Err> { match s { // ... existing arms ... - _ => Err(()), + _ => Err(format!("unknown transaction kind: {s}")), } } }engine/src/fetch_contract.rs (3)
97-116: Inefficient:network_fetchfetches all releases to find one version.When fetching a specific version, this function retrieves all releases from GitHub and searches through them. For repositories with many releases, this is inefficient. Consider using the GitHub API's direct release-by-tag endpoint:
GET /repos/{owner}/{repo}/releases/tags/{tag}.
26-61:all()silently continues on storage errors, making failures non-obvious.When
store_contract_by_versionfails, the function logs an error but continues processing and ultimately returnsOk(()). This makes it difficult for callers to know if all contracts were stored successfully. Consider tracking failed versions and returning a partial failure result or at least a warning count.
20-24:ContractKeystruct is unused and should be removed or annotated.Verification confirms the struct is defined at line 20 but not referenced anywhere in the codebase. Remove it or add a
#[allow(dead_code)]annotation with a TODO comment explaining its intended purpose.engine/src/runner.rs (1)
139-152: Hardcoded mainnet version mappings may need updates.The
Defaultimplementation contains specific block heights and versions for mainnet. Consider loading these from configuration or documenting the update process when new versions are deployed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
Cargo.lockis excluded by!**/*.locketc/res/aurora-engine-3.6.4-ext-connector.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.6.4.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.7.0-ext-connector.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.7.0.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.0-ext-connector.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.0.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.1-ext-connector.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.1.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.2-ext-connector.wasmis excluded by!**/*.wasmetc/res/aurora-engine-3.9.2.wasmis excluded by!**/*.wasm
📒 Files selected for processing (26)
Cargo.toml(2 hunks)default_config.json(1 hunks)engine/Cargo.toml(1 hunks)engine/src/batch_tx_processing.rs(0 hunks)engine/src/fetch_contract.rs(1 hunks)engine/src/lib.rs(3 hunks)engine/src/runner.rs(1 hunks)engine/src/storage_ext.rs(1 hunks)engine/src/sync.rs(14 hunks)engine/src/tests.rs(4 hunks)engine/src/tracing/lib.rs(2 hunks)etc/integration-tests/src/refiner_utils.rs(1 hunks)nearcore_config.json(1 hunks)refiner-app/Cargo.toml(1 hunks)refiner-app/src/cli.rs(1 hunks)refiner-app/src/config.rs(2 hunks)refiner-app/src/main.rs(3 hunks)refiner-app/src/socket.rs(19 hunks)refiner-lib/src/kind.rs(1 hunks)refiner-lib/src/lib.rs(1 hunks)refiner-lib/src/metrics.rs(3 hunks)refiner-lib/src/near_stream.rs(5 hunks)refiner-lib/src/refiner_inner.rs(17 hunks)refiner-lib/src/storage.rs(3 hunks)refiner-types/src/lib.rs(1 hunks)refiner-types/src/source_config.rs(1 hunks)
💤 Files with no reviewable changes (1)
- engine/src/batch_tx_processing.rs
🧰 Additional context used
🧬 Code graph analysis (7)
engine/src/runner.rs (1)
engine/src/storage_ext.rs (1)
store_contract_by_version(14-22)
refiner-lib/src/refiner_inner.rs (4)
refiner-lib/src/legacy.rs (1)
decode_submit_result(87-109)refiner-lib/src/metrics.rs (1)
record_metric(254-405)refiner-lib/src/utils.rs (2)
as_h256(8-12)try_from(24-62)refiner-lib/src/kind.rs (1)
from_str(61-115)
engine/src/fetch_contract.rs (1)
engine/src/storage_ext.rs (2)
store_contract_by_version(14-22)store_contract(8-12)
engine/src/tracing/lib.rs (1)
engine/src/storage_ext.rs (1)
apply_contract(36-56)
engine/src/tests.rs (2)
engine/src/lib.rs (3)
new(27-40)sync(48-48)consume_near_block(43-56)engine/src/storage_ext.rs (1)
load_from_file(84-104)
engine/src/storage_ext.rs (1)
engine/src/lib.rs (1)
new(27-40)
engine/src/sync.rs (3)
engine/src/lib.rs (1)
sync(48-48)engine/src/tracing/lib.rs (1)
sync(47-47)engine/src/storage_ext.rs (1)
apply_contract(36-56)
🔇 Additional comments (33)
nearcore_config.json (1)
16-16: LGTM!Trailing newline removal is a minor formatting change with no functional impact.
refiner-app/Cargo.toml (1)
31-34: LGTM!Adding
near-primitives-coreas a dev-dependency with workspace resolution is appropriate for development/testing needs without affecting runtime behavior.default_config.json (1)
17-21: New contract source configuration looks good.The GitHub-based contract source configuration is well-structured with correct API endpoint and repository format.
Consider documenting that unauthenticated GitHub API requests are rate-limited to 60 requests/hour. If the refiner makes frequent contract version checks, users may need to configure authentication or implement caching to avoid rate limit issues in production.
refiner-lib/src/storage.rs (2)
11-13: Simplified initialization flow.The direct call to
migrate_block_hashstreamlines the initialization by removing redundant conditional checks.
48-48: Consistent use ofopen_ensure_account_idafter migration.Good consistency in using the same storage opening method post-migration.
etc/integration-tests/src/refiner_utils.rs (1)
81-94: Contract path verification confirms directory exists with expected contract files.The
etc/resdirectory exists and contains 10 WASM contract files (aurora-engine versions 3.6.4, 3.7.0, 3.9.0, 3.9.1, and 3.9.2, with and without ext-connector variants). The hardcoded path in the code is correct and points to the appropriate location for contract artifacts.refiner-app/src/config.rs (1)
2-2: LGTM: Backward-compatible config extension.The addition of the optional
contract_sourcefield is backward compatible and properly typed usingContractSourcefromrefiner-types.Also applies to: 15-15
refiner-types/src/lib.rs (1)
9-9: LGTM: Clean module addition.The public export of
source_configis straightforward and extends the crate's API appropriately.engine/src/tests.rs (2)
94-94: LGTM: Cleaner pattern matching.The updated pattern matching for
TransactionExecutionResult::Submitdirectly extracts the value instead of unwrapping, which is more idiomatic and clearer.Also applies to: 212-212
235-240: Contract WASM file verification passed—no issues found.The required file
etc/res/aurora-engine-3.7.0.wasmexists in the repository at the expected location. Thestorage_ext::load_from_file("3.7.0", None)function correctly uses the default prefix"etc/res"and constructs the path asetc/res/aurora-engine-3.7.0.wasm, which is present (1.56 MB). Test initialization will succeed without issues.refiner-lib/src/lib.rs (1)
12-12: Thekindmodule is actively used and not dead code.The module is imported and used extensively in two internal modules:
metrics.rs(line 4): importsTxKindand uses it inrecord_metric()function with comprehensive pattern matching across all transaction kindsrefiner_inner.rs(line 1): importsTxKindand uses it for transaction type identification, parsing, and routing throughout the refiner logicThe private declaration is appropriate since this module is an internal implementation detail used only within the crate.
refiner-lib/src/metrics.rs (1)
254-404: LGTM!The migration from
TransactionKindTagtoTxKindis comprehensive and all variants are correctly mapped to their corresponding metric counters. The newDeployErc20Callbackvariant is properly wired.refiner-app/src/main.rs (1)
118-125: Verify the error handling strategy for contract fetch failure.When
fetch_contract::allfails, the error is logged but execution continues. If the fetched contracts are required for correct operation, this could lead to confusing failures downstream. Consider whether this should be a fatal error or if there's a valid fallback path.refiner-lib/src/near_stream.rs (2)
74-81: LGTM!The storage access pattern change from a write guard to a mutable reference aligns well with the PR's refactoring from
Arc<RwLock<Storage>>to directStorageownership.
602-606: LGTM!The test context properly initializes the engine storage with contract code using the new runner API. The hardcoded version "3.7.0" is appropriate for deterministic test fixtures.
refiner-lib/src/kind.rs (1)
3-56: LGTM!The
TxKindenum provides a comprehensive and type-safe representation of all transaction kinds, with proper derives for common traits. The variants are well-organized and cover the full transaction taxonomy.engine/src/storage_ext.rs (1)
84-104: LGTM with minor note on fallback logic.The recursive fallback to
../etc/resfor tests is a reasonable pattern. Theoverride_prefix.clone()on line 88 is minor but could useas_ref()pattern instead.refiner-lib/src/refiner_inner.rs (5)
1-42: LGTM - Clean refactor fromTransactionKindTagtoTxKind.The imports and type changes are consistent. The switch to
FtOnTransferArgsandTransactionExecutionResultaligns with the broader PR changes.
383-530: LGTM -normalize_outputcorrectly updated to useTxKindvariants.The function logic is preserved with appropriate variant matching for
TxKind::Submit,TxKind::Call,TxKind::Deploy,TxKind::SubmitWithArgs,TxKind::WithdrawWnearToRouter, andTxKind::FtOnTransfer.
600-607: VerifyTxKind::from_strreturnsUnknownfor unrecognized methods.Line 600 uses
unwrap_or(TxKind::Unknown)which safely handles parsing failures. The warning on line 605 correctly logs unknown methods.
972-1052: LGTM -FtOnTransferhandling correctly usesFtOnTransferArgs.The parsing at line 976 and subsequent usage at lines 1004, 1008-1011 properly handle the new
FtOnTransferArgstype.
1139-1192: LGTM -determine_ft_on_transfer_recipientupdated for new types.The function correctly uses
FtOnTransferArgsandFtTransferMessageData::try_fromat line 1148. The error handling with fallback toAddress::zero()is preserved.engine/src/lib.rs (1)
19-24: Based on my verification, I can now provide the rewritten review comment:
Storage ownership correctly changed from Arc<RwLock> to direct
Storagewith proper concurrent access handling.The async
consume_near_blockwrapper immediately delegates tosync::consume_near_blockwithout holding the&mutreference across await points, so there are no concurrency issues. Concurrent access is properly managed via theStorage::share()method used by callers inrefiner-appfor task distribution. This architectural pattern is sound and appropriately documented through usage.refiner-app/src/socket.rs (4)
18-48: LGTM! Clean refactor to owned Storage with share() pattern.The transition from
Arc<RwLock<Storage>>to directStorageownership with.share()for task distribution is well-implemented. The socket server correctly clones thelinkand shares the storage for each incoming connection.
50-136: Connection handler correctly propagates storage and link context.The per-message handling properly shares storage and clones the link for each request. The JSON-RPC error handling remains intact.
139-173: Unused_linkparameter prepared for future use.The
_linkparameter is passed through but not yet utilized in these handlers. This is acceptable for incremental development, and the underscore prefix correctly suppresses unused warnings.
224-288: Test infrastructure properly updated for new ownership model.The
TestStoragestruct and tests are correctly updated to useStoragedirectly with.share(), andNoneis appropriately passed for thelinkparameter in tests.engine/src/sync.rs (6)
76-100: Verify fail-open behavior is intentional for contract reload errors.The error handling logs failures but continues processing. While this prevents sync from blocking on contract issues:
set_codefailure (line 76-81): Logs error, continuesapply_contractfailure (line 84-92): Logs error, continuesget_versionfailure (line 93-99): Logs debug, continuesThis fail-open pattern could leave the runner with stale or no code. Verify this is the intended behavior for production use.
218-221:trace_kind: Nonedisables tracing for all transactions.The
trace_kindfield is set toNonefor all transaction messages. This appears intentional as part of the refactoring, but verify that tracing functionality is properly triggered elsewhere when needed (e.g., viadebug_traceTransactionRPC).
486-506: Batch processing uses<true>for diff caching.The const generic pattern
execute_transaction_message::<true>for batch actions (lines 489, 496) enables diff caching, which is then collected viatake_cached_diff()at line 502. This is a clever optimization for cumulative diff tracking in batch transactions.
431-443: SimplifiedSingleParsedActionremoves redundantraw_input.The struct now only carries
transaction_kindanddeposit, withTransactionKind::new()andTransactionKind::unknown()handling the internal details. This is a clean API improvement.
634-651:parse_actioncleanly delegates toTransactionKind::new.The refactored function properly constructs
TransactionKindwith method name, args, and promise data, returning just the kind and deposit. This aligns with the new API surface.
312-330: Result validation refactored with cleaner match pattern.The nested match for validating
SubmitResultis well-structured with appropriate warning logs for mismatches and deserialization failures.
| aurora-engine = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std", "tracing", "log", "impl-serde"] } | ||
| aurora-engine-transactions = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std", "impl-serde"] } | ||
| aurora-engine-types = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std", "impl-serde"] } | ||
| aurora-engine-sdk = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std"] } | ||
| aurora-engine-modexp = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std"] } | ||
| aurora-engine-hashchain = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std"] } | ||
| engine-standalone-storage = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false } | ||
| engine-standalone-tracing = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["impl-serde"] } |
There was a problem hiding this comment.
Critical: Using feature branch instead of stable release.
All aurora-engine dependencies reference branch = "feat/wasm-tracing" instead of a stable tagged release. Feature branches are unstable, can change without notice, and break build reproducibility.
For production readiness, consider one of these approaches:
- Wait for the wasm-tracing feature to be merged and use a tagged release
- Use a specific commit hash instead of a branch name
- Document that this is experimental and pin to a specific commit
Apply this pattern for stable builds:
-aurora-engine = { git = "https://github.com/aurora-is-near/aurora-engine.git", branch = "feat/wasm-tracing", default-features = false, features = ["std", "tracing", "log", "impl-serde"] }
+aurora-engine = { git = "https://github.com/aurora-is-near/aurora-engine.git", rev = "SPECIFIC_COMMIT_HASH", default-features = false, features = ["std", "tracing", "log", "impl-serde"] }(Apply the same pattern to all aurora-engine-* dependencies)
Committable suggestion skipped: line range outside the PR's diff.
| [patch."https://github.com/aurora-is-near/aurora-engine.git"] | ||
| aurora-engine = { path = "../aurora-engine/engine" } | ||
| aurora-engine-transactions = { path = "../aurora-engine/engine-transactions" } | ||
| aurora-engine-types = { path = "../aurora-engine/engine-types" } | ||
| aurora-engine-sdk = { path = "../aurora-engine/engine-sdk" } | ||
| aurora-engine-modexp = { path = "../aurora-engine/engine-modexp" } | ||
| aurora-engine-hashchain = { path = "../aurora-engine/engine-hashchain" } | ||
| engine-standalone-storage = { path = "../aurora-engine/engine-standalone-storage" } | ||
| engine-standalone-tracing = { path = "../aurora-engine/engine-standalone-tracing" } |
There was a problem hiding this comment.
Critical: Local path patches break portability.
The [patch] section overrides all aurora-engine dependencies with local filesystem paths (../aurora-engine/*). This will break for:
- Other developers without the same directory structure
- CI/CD pipelines
- Production deployments
- Anyone cloning the repository
Solutions:
- Remove the patch section before merging (recommended for production):
-[patch."https://github.com/aurora-is-near/aurora-engine.git"]
-aurora-engine = { path = "../aurora-engine/engine" }
-aurora-engine-transactions = { path = "../aurora-engine/engine-transactions" }
-aurora-engine-types = { path = "../aurora-engine/engine-types" }
-aurora-engine-sdk = { path = "../aurora-engine/engine-sdk" }
-aurora-engine-modexp = { path = "../aurora-engine/engine-modexp" }
-aurora-engine-hashchain = { path = "../aurora-engine/engine-hashchain" }
-engine-standalone-storage = { path = "../aurora-engine/engine-standalone-storage" }
-engine-standalone-tracing = { path = "../aurora-engine/engine-standalone-tracing" }- Or document as dev-only and add to
.cargo/config.tomlinstead:- Move patches to a local
.cargo/config.toml(gitignored) - Add a comment in the main Cargo.toml explaining how developers can set up local patches for development
- Ensure CI/CD uses the git dependencies
- Move patches to a local
Since this is a draft PR, you likely want option 2 for development and option 1 before final merge.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [patch."https://github.com/aurora-is-near/aurora-engine.git"] | |
| aurora-engine = { path = "../aurora-engine/engine" } | |
| aurora-engine-transactions = { path = "../aurora-engine/engine-transactions" } | |
| aurora-engine-types = { path = "../aurora-engine/engine-types" } | |
| aurora-engine-sdk = { path = "../aurora-engine/engine-sdk" } | |
| aurora-engine-modexp = { path = "../aurora-engine/engine-modexp" } | |
| aurora-engine-hashchain = { path = "../aurora-engine/engine-hashchain" } | |
| engine-standalone-storage = { path = "../aurora-engine/engine-standalone-storage" } | |
| engine-standalone-tracing = { path = "../aurora-engine/engine-standalone-tracing" } |
| aurora-engine-sdk.workspace = true | ||
| aurora-refiner-types = { path = "../refiner-types" } | ||
| borsh.workspace = true | ||
| bytes.workspace = true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if new dependencies are actually used in engine crate
echo "=== Checking bytes usage ==="
rg -n --type=rust '\buse\s+bytes\b' engine/src/
echo "=== Checking futures usage ==="
rg -n --type=rust '\buse\s+futures\b' engine/src/
echo "=== Checking reqwest usage ==="
rg -n --type=rust '\buse\s+reqwest\b' engine/src/
echo "=== Checking sha2 usage ==="
rg -n --type=rust '\buse\s+sha2\b' engine/src/
echo "=== Checking thiserror usage ==="
rg -n --type=rust '\buse\s+thiserror\b|\bError\]' engine/src/
echo "=== Checking near-jsonrpc-client usage ==="
rg -n --type=rust '\buse\s+near_jsonrpc_client\b' engine/src/
echo "=== Checking near-vm-runner usage ==="
rg -n --type=rust '\buse\s+near_vm_runner\b' engine/src/Repository: aurora-is-near/borealis-engine-lib
Length of output: 691
🏁 Script executed:
#!/bin/bash
# Check the actual Cargo.toml for all new dependencies
echo "=== engine/Cargo.toml dependencies section ==="
sed -n '24,50p' engine/Cargo.toml
echo -e "\n=== Checking near-vm-runner with broader pattern ==="
rg -n --type=rust 'near_vm_runner' engine/src/
echo -e "\n=== Checking tracing usage ==="
rg -n --type=rust '\buse\s+tracing\b' engine/src/Repository: aurora-is-near/borealis-engine-lib
Length of output: 1023
🏁 Script executed:
#!/bin/bash
# Check remaining dependencies
echo "=== Checking hex usage ==="
rg -n --type=rust '\bhex\b' engine/src/ | head -20
echo -e "\n=== Checking near-crypto usage ==="
rg -n --type=rust 'near_crypto' engine/src/
echo -e "\n=== Checking near-jsonrpc-primitives usage ==="
rg -n --type=rust 'near_jsonrpc_primitives' engine/src/
echo -e "\n=== Checking near-parameters usage ==="
rg -n --type=rust 'near_parameters' engine/src/
echo -e "\n=== Checking near-primitives-core usage ==="
rg -n --type=rust 'near_primitives_core' engine/src/
echo -e "\n=== Double-check near-vm-runner in all files ==="
find engine/src -name "*.rs" -exec grep -l "near_vm_runner" {} \;Repository: aurora-is-near/borealis-engine-lib
Length of output: 1772
🏁 Script executed:
#!/bin/bash
# Check for indirect usage patterns and macro usage
echo "=== Searching for CryptoHash or crypto-related usage ==="
rg -n --type=rust 'CryptoHash|crypto' engine/src/ | grep -i near | head -10
echo -e "\n=== Check if near-vm-runner might be used through other crates ==="
rg -n --type=rust 'VMContext|ComputationMetrics|near_vm' engine/src/
echo -e "\n=== Check Cargo.toml for feature gates or indirect dependencies ==="
grep -A 5 -B 5 'near-vm-runner\|near-crypto\|near-parameters' engine/Cargo.toml
echo -e "\n=== Check for use of these in conditional compilation ==="
rg -n --type=rust '#\[cfg' engine/src/ | grep -i near
echo -e "\n=== Final verification: list all Rust files and check content ==="
find engine/src -name "*.rs" | xargs wc -l | sort -n | tail -10Repository: aurora-is-near/borealis-engine-lib
Length of output: 1430
Remove unused dependencies: near-crypto, near-parameters, and near-vm-runner.
Verification confirms that most new dependencies are properly used (bytes, futures, reqwest, sha2, thiserror, tracing, hex, near-jsonrpc-client, near-jsonrpc-primitives, and near-primitives-core all have confirmed usage). However, three dependencies were added but are never imported or used anywhere in the engine crate:
near-cryptonear-parametersnear-vm-runner
These should be removed from engine/Cargo.toml unless they serve a purpose not visible in the code (e.g., required for transitive dependencies or build artifacts).
🤖 Prompt for AI Agents
In engine/Cargo.toml around line 24, three dependencies (near-crypto,
near-parameters, near-vm-runner) were added but are not used anywhere in the
crate; remove their entries from Cargo.toml, then run cargo check/cargo build
(or cargo tree) to confirm no breakage or transitive requirement; if a
dependency is actually required for a transitive or build-time use, restore it
with a short comment explaining why it must remain.
| pub async fn fetch_asset(asset: &GitHubAsset) -> Result<Vec<u8>, FetchError> { | ||
| let user_agent = concat!("borealis-engine-lib/", env!("CARGO_PKG_VERSION")); | ||
| let client = reqwest::Client::builder().user_agent(user_agent).build()?; | ||
| if asset.size > 64 * 1024 * 1024 { | ||
| panic!("Asset too large: {}", asset.size); | ||
| } | ||
| let mut response = client | ||
| .get(asset.browser_download_url.clone()) | ||
| .header("Accept", "Application/octet-stream") | ||
| .send() | ||
| .await?; | ||
| if !response.status().is_success() { | ||
| return Err(FetchError::Status(response.status())); | ||
| } | ||
| let mut bytes = BytesMut::default(); | ||
| while let Some(chunk) = response.chunk().await? { | ||
| bytes.extend_from_slice(&chunk); | ||
| } | ||
| let bytes = bytes.to_vec(); | ||
| if let Some(digest) = &asset.digest { | ||
| let result = Sha256::default().chain(&bytes).finalize_fixed(); | ||
| let computed = hex::encode(result); | ||
| if &computed != digest { | ||
| panic!( | ||
| "Digest mismatch: expected {digest}, got {computed} for asset {}", | ||
| asset.name | ||
| ); | ||
| } | ||
| } | ||
| Ok(bytes) |
There was a problem hiding this comment.
Replace panic! with error returns for production resilience.
Using panic! for asset size validation (line 164) and digest mismatch (lines 183-186) will crash the entire application on malformed or malicious release data. These should return proper errors to allow graceful handling.
+#[derive(Debug, Error)]
+pub enum AssetError {
+ #[error("Asset too large: {0} bytes")]
+ TooLarge(u64),
+ #[error("Digest mismatch: expected {expected}, got {computed}")]
+ DigestMismatch { expected: String, computed: String },
+}
pub async fn fetch_asset(asset: &GitHubAsset) -> Result<Vec<u8>, FetchError> {
let user_agent = concat!("borealis-engine-lib/", env!("CARGO_PKG_VERSION"));
let client = reqwest::Client::builder().user_agent(user_agent).build()?;
if asset.size > 64 * 1024 * 1024 {
- panic!("Asset too large: {}", asset.size);
+ return Err(FetchError::AssetTooLarge(asset.size));
}
// ... fetching logic ...
if let Some(digest) = &asset.digest {
let result = Sha256::default().chain(&bytes).finalize_fixed();
let computed = hex::encode(result);
if &computed != digest {
- panic!(
- "Digest mismatch: expected {digest}, got {computed} for asset {}",
- asset.name
- );
+ return Err(FetchError::DigestMismatch {
+ expected: digest.clone(),
+ computed,
+ asset_name: asset.name.clone(),
+ });
}
}
Ok(bytes)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In engine/src/fetch_contract.rs around lines 160–189, replace the two panic!
calls (asset size check and digest mismatch) with proper Err(FetchError::...)
returns so the function returns a Result error instead of aborting the process;
add or use FetchError variants such as AssetTooLarge(size: u64) for the size
check and DigestMismatch { expected: String, got: String, name: String } for the
digest mismatch, return Err(...) with those details where the panics are now,
and keep the rest of the async flow unchanged so callers can handle these errors
gracefully.
| pub async fn run<S>(mut contract_update: S, shared_storage: Arc<RwLock<Storage>>) | ||
| where | ||
| S: Stream<Item = (String, Vec<u8>)> + Unpin, | ||
| { | ||
| while let Some((version, code)) = contract_update.next().await { | ||
| let storage = shared_storage.write().expect("must not crash on lock"); | ||
| if let Err(err) = storage_ext::store_contract_by_version(&storage, &version, &code) { | ||
| tracing::error!( | ||
| err = format!("{err:?}"), | ||
| new_version = &version, | ||
| "Failed to store updated contract", | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
expect() on RwLock can crash on poisoned lock.
Line 14 uses .expect("must not crash on lock") which will panic if another thread panicked while holding the lock. Consider using .write().unwrap_or_else(|e| e.into_inner()) to recover from poisoned locks, or propagate the error gracefully.
pub async fn run<S>(mut contract_update: S, shared_storage: Arc<RwLock<Storage>>)
where
S: Stream<Item = (String, Vec<u8>)> + Unpin,
{
while let Some((version, code)) = contract_update.next().await {
- let storage = shared_storage.write().expect("must not crash on lock");
+ let storage = shared_storage.write().unwrap_or_else(|e| {
+ tracing::warn!("Lock was poisoned, recovering");
+ e.into_inner()
+ });
if let Err(err) = storage_ext::store_contract_by_version(&storage, &version, &code) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn run<S>(mut contract_update: S, shared_storage: Arc<RwLock<Storage>>) | |
| where | |
| S: Stream<Item = (String, Vec<u8>)> + Unpin, | |
| { | |
| while let Some((version, code)) = contract_update.next().await { | |
| let storage = shared_storage.write().expect("must not crash on lock"); | |
| if let Err(err) = storage_ext::store_contract_by_version(&storage, &version, &code) { | |
| tracing::error!( | |
| err = format!("{err:?}"), | |
| new_version = &version, | |
| "Failed to store updated contract", | |
| ); | |
| } | |
| } | |
| } | |
| pub async fn run<S>(mut contract_update: S, shared_storage: Arc<RwLock<Storage>>) | |
| where | |
| S: Stream<Item = (String, Vec<u8>)> + Unpin, | |
| { | |
| while let Some((version, code)) = contract_update.next().await { | |
| let storage = shared_storage.write().unwrap_or_else(|e| { | |
| tracing::warn!("Lock was poisoned, recovering"); | |
| e.into_inner() | |
| }); | |
| if let Err(err) = storage_ext::store_contract_by_version(&storage, &version, &code) { | |
| tracing::error!( | |
| err = format!("{err:?}"), | |
| new_version = &version, | |
| "Failed to store updated contract", | |
| ); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In engine/src/runner.rs around lines 9 to 23, the call to
shared_storage.write().expect("must not crash on lock") can panic on a poisoned
RwLock; replace the expect with poison-aware handling — either recover the inner
lock via .write().unwrap_or_else(|e| e.into_inner()) before using storage, or
propagate the error/result so the task can log and continue; ensure subsequent
code uses the recovered or returned lock value and does not call expect so a
poisoned lock no longer causes a panic.
| #[test] | ||
| fn test_switch_contract() { | ||
| tracing_subscriber::fmt::try_init().unwrap_or_default(); | ||
|
|
||
| let dir = fs::read_dir("src/res/deploy").unwrap(); | ||
|
|
||
| let mut data_id_mapping = lru::LruCache::new(NonZeroUsize::new(1000).unwrap()); | ||
| let mut outcomes_map = HashMap::new(); | ||
| let chain_id = aurora_engine_types::types::u256_to_arr(&(1313161554.into())); | ||
|
|
||
| let mut test_context = | ||
| TestContext::load_snapshot("src/res/contract.aurora.block66381606.minimal.json"); | ||
| test_context.engine_account_id = "borealis.testnet".parse().unwrap(); | ||
|
|
||
| let blocks = dir.map(|item| { | ||
| let path = item.unwrap().path(); | ||
| let file = fs::File::open(path).unwrap(); | ||
| serde_json::from_reader::<_, NEARBlock>(file).unwrap() | ||
| }); | ||
| let versions = ["3.6.4", "3.7.0", "3.9.0", "3.9.1", "3.9.2"]; | ||
| for (block, expected_version) in blocks.zip(versions) { | ||
| crate::sync::consume_near_block::<AuroraModExp>( | ||
| &mut test_context.storage, | ||
| &block, | ||
| &mut data_id_mapping, | ||
| &test_context.engine_account_id, | ||
| chain_id, | ||
| Some(&mut outcomes_map), | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| // the contract version is updated | ||
| let actual_version = test_context | ||
| .storage | ||
| .runner_mut() | ||
| .get_version_at_wrapper() | ||
| .unwrap(); | ||
| let actual_version = actual_version.as_str().trim_end(); | ||
|
|
||
| assert_eq!(actual_version, expected_version); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking test resources ==="
# Check from repo root - paths are likely relative to engine/ since test is in engine/src/tests.rs
if [ -d "engine/src/res/deploy" ]; then
echo "✓ deploy directory found at engine/src/res/deploy"
echo "Files in deploy directory:"
ls -lh engine/src/res/deploy/ 2>/dev/null | head -20
else
echo "✗ deploy directory NOT found at engine/src/res/deploy"
fi
echo ""
if [ -f "engine/src/res/contract.aurora.block66381606.minimal.json" ]; then
echo "✓ contract snapshot found at engine/src/res/contract.aurora.block66381606.minimal.json"
ls -lh engine/src/res/contract.aurora.block66381606.minimal.json
else
echo "✗ contract snapshot NOT found at engine/src/res/contract.aurora.block66381606.minimal.json"
fi
echo ""
echo "=== Checking directory structure of engine/src/res ==="
if [ -d "engine/src/res" ]; then
find engine/src/res -type f | head -20
else
echo "engine/src/res directory doesn't exist"
fi
echo ""
echo "=== Checking test file to understand path context ==="
head -30 engine/src/tests.rsRepository: aurora-is-near/borealis-engine-lib
Length of output: 2370
🏁 Script executed:
#!/bin/bash
# Check how files are enumerated by fs::read_dir
# Need to understand if order is guaranteed
echo "=== Analyzing the test logic ==="
sed -n '14,55p' engine/src/tests.rs
echo ""
echo "=== Checking file listing order ==="
echo "Files in deploy directory (natural order):"
ls -1 engine/src/res/deploy/
echo ""
echo "=== Checking if files are sorted ==="
ls -1 engine/src/res/deploy/ | sortRepository: aurora-is-near/borealis-engine-lib
Length of output: 1861
🏁 Script executed:
#!/bin/bash
echo "=== Checking Rust documentation for fs::read_dir order guarantee ==="
# Search for other test patterns in the file
echo "Looking for other test patterns and usages:"
grep -n "fs::read_dir\|#\[test\]" engine/src/tests.rs | head -30
echo ""
echo "=== Checking if this pattern is used elsewhere ==="
rg -n "fs::read_dir" engine/src/ -A 5
echo ""
echo "=== Checking test file line count ==="
wc -l engine/src/tests.rsRepository: aurora-is-near/borealis-engine-lib
Length of output: 813
Based on my verification, I can now provide the rewritten review comment:
Test resources exist but implementation lacks deterministic ordering.
The test resources referenced by test_switch_contract are present in the repository:
engine/src/res/deploy/contains exactly 5 JSON files (217143860.json, 217143880.json, 217143901.json, 217143921.json, 217143934.json)engine/src/res/contract.aurora.block66381606.minimal.jsonexists
However, the test has a subtle fragility: fs::read_dir() does not guarantee file enumeration order across different platforms or filesystems. While the files currently enumerate in numeric order, this is undocumented behavior. If:
- Additional files are added to
src/res/deploy, the.zip()silently truncates and the test passes with fewer blocks than intended - The filesystem reorders entries, assertions fail with mismatched versions
Consider explicitly sorting the directory entries to ensure deterministic behavior:
let dir = fs::read_dir("src/res/deploy").unwrap();
+let mut entries: Vec<_> = dir.collect::<Result<Vec<_>, _>>().unwrap();
+entries.sort_by_key(|e| e.path());
-let blocks = dir.map(|item| {
+let blocks = entries.into_iter().map(|item| {
let path = item.unwrap().path();This makes test behavior independent of filesystem ordering and catches unexpected files through assertion failures rather than silent truncation.
🤖 Prompt for AI Agents
In engine/src/tests.rs around lines 14 to 55, the test reads files from
src/res/deploy using fs::read_dir() which yields nondeterministic ordering and
then zips with expected versions causing brittle behavior and silent truncation;
fix by collecting the DirEntry iterator into a Vec, sort the entries
deterministically (e.g., by file name or parsed numeric basename), assert the
collected length equals the expected number of blocks, then map the sorted
entries to blocks (opening and deserializing each) and iterate them in order so
the test is stable and will fail loudly if files are missing or extra.
| if let Err(err) = storage_ext::apply_contract(storage, height, tx_msg.position, None, None) { | ||
| tracing::error!( | ||
| tx_hash = format!("{tx_hash:#x}"), | ||
| height, | ||
| pos = tx_msg.position, | ||
| err = format!("{err:?}"), | ||
| "Failed to apply contract for tracing", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider returning an error when contract application fails.
The tracing continues even if apply_contract fails, which could produce incorrect or misleading trace results. If the contract cannot be loaded, the trace output may not reflect the actual execution context.
- if let Err(err) = storage_ext::apply_contract(storage, height, tx_msg.position, None, None) {
- tracing::error!(
- tx_hash = format!("{tx_hash:#x}"),
- height,
- pos = tx_msg.position,
- err = format!("{err:?}"),
- "Failed to apply contract for tracing",
- );
- }
+ storage_ext::apply_contract(storage, height, tx_msg.position, None, None).map_err(|err| {
+ tracing::error!(
+ tx_hash = format!("{tx_hash:#x}"),
+ height,
+ pos = tx_msg.position,
+ err = format!("{err:?}"),
+ "Failed to apply contract for tracing",
+ );
+ engine_standalone_storage::Error::NoBlockHeight { height }
+ })?;Committable suggestion skipped: line range outside the PR's diff.
| /// Path to contract wasm | ||
| #[clap(short, long)] | ||
| pub contract_path: Option<String>, |
There was a problem hiding this comment.
Critical: CLI short flag conflict.
Both config_path (line 7) and contract_path (line 10) use #[clap(short, long)], which defaults to the first letter of the field name. This creates a conflict where both would try to use -c as the short flag.
Apply this diff to use a different short flag for contract_path:
/// Path to contract wasm
- #[clap(short, long)]
+ #[clap(short = 'w', long)]
pub contract_path: Option<String>,Alternatively, remove the short flag if it's not essential:
/// Path to contract wasm
- #[clap(short, long)]
+ #[clap(long)]
pub contract_path: Option<String>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Path to contract wasm | |
| #[clap(short, long)] | |
| pub contract_path: Option<String>, | |
| /// Path to contract wasm | |
| #[clap(short = 'w', long)] | |
| pub contract_path: Option<String>, |
🤖 Prompt for AI Agents
In refiner-app/src/cli.rs around lines 9 to 11, the #[clap(short, long)] on
contract_path conflicts with config_path because both default to -c; change the
attribute to either specify a unique short flag (e.g., use a different
single-character short like -w or -r) or remove the short flag entirely (keep
only long) depending on whether a short alias is needed, and update any
docs/tests that reference the old -c short flag.
| pub static ref TRANSACTION_TYPE_DEPLOY_ERC20_CALLBACK_TOKEN: IntCounter = counter( | ||
| "refiner_tx_type_deploy_erc20_callback_token", | ||
| "Number of transactions of type: refiner_tx_type_deploy_erc20_callback_token" | ||
| ); |
There was a problem hiding this comment.
Fix the metric description.
The description repeats the metric name instead of providing a meaningful description like the other counters.
pub static ref TRANSACTION_TYPE_DEPLOY_ERC20_CALLBACK_TOKEN: IntCounter = counter(
"refiner_tx_type_deploy_erc20_callback_token",
- "Number of transactions of type: refiner_tx_type_deploy_erc20_callback_token"
+ "Number of transactions of type: deploy_erc20_callback_token"
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub static ref TRANSACTION_TYPE_DEPLOY_ERC20_CALLBACK_TOKEN: IntCounter = counter( | |
| "refiner_tx_type_deploy_erc20_callback_token", | |
| "Number of transactions of type: refiner_tx_type_deploy_erc20_callback_token" | |
| ); | |
| pub static ref TRANSACTION_TYPE_DEPLOY_ERC20_CALLBACK_TOKEN: IntCounter = counter( | |
| "refiner_tx_type_deploy_erc20_callback_token", | |
| "Number of transactions of type: deploy_erc20_callback_token" | |
| ); |
🤖 Prompt for AI Agents
In refiner-lib/src/metrics.rs around lines 67 to 70, the metric's help string
simply repeats the metric name; replace the second argument with a meaningful
description consistent with other counters, e.g. "Number of transactions of
type: deploy ERC20 callback token" (or similar human-readable text) so the
counter documents what it measures rather than repeating its identifier.
| #[derive(Deserialize, Clone, Debug, PartialEq, Eq)] | ||
| #[serde(tag = "kind", rename_all = "snake_case")] | ||
| pub enum ContractSource { | ||
| Github { base_url: String, repo: String }, |
There was a problem hiding this comment.
Major: Missing validation for URL and repository fields.
The base_url and repo fields accept arbitrary strings without validation. This could lead to:
- Invalid URLs causing runtime failures
- Path traversal attacks if
repocontains../sequences - SSRF vulnerabilities if
base_urlpoints to internal resources
Consider adding validation:
+use std::fmt;
+
#[derive(Deserialize, Clone, Debug, PartialEq, Eq)]
#[serde(tag = "kind", rename_all = "snake_case")]
pub enum ContractSource {
- Github { base_url: String, repo: String },
+ Github {
+ #[serde(deserialize_with = "deserialize_validated_url")]
+ base_url: String,
+ #[serde(deserialize_with = "deserialize_validated_repo")]
+ repo: String,
+ },
+}
+
+fn deserialize_validated_url<'de, D>(deserializer: D) -> Result<String, D::Error>
+where
+ D: serde::Deserializer<'de>,
+{
+ let url = String::deserialize(deserializer)?;
+ // Validate URL format
+ url::Url::parse(&url)
+ .map_err(|e| serde::de::Error::custom(format!("Invalid URL: {}", e)))?;
+ Ok(url)
+}
+
+fn deserialize_validated_repo<'de, D>(deserializer: D) -> Result<String, D::Error>
+where
+ D: serde::Deserializer<'de>,
+{
+ let repo = String::deserialize(deserializer)?;
+ // Validate repo format (e.g., "owner/repo")
+ if !repo.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_' || c == '/') {
+ return Err(serde::de::Error::custom("Invalid repository format"));
+ }
+ if repo.contains("..") {
+ return Err(serde::de::Error::custom("Path traversal not allowed"));
+ }
+ Ok(repo)
}Note: You'll need to add url = "2" to your dependencies.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In refiner-types/src/source_config.rs around line 6, the Github struct currently
accepts arbitrary strings for base_url and repo; add input validation by
requiring base_url be parsed with the url crate (add url = "2" to Cargo.toml)
and only accept URLs with scheme "http" or "https" and a non-empty host
(optionally reject private/internal IPs if needed). For repo, trim and validate
it matches an allowed pattern (e.g., owner/repo or repo name only, characters
limited to [A-Za-z0-9_.-] and single slash, no leading/trailing slashes, and
explicitly reject any ".." or path traversal sequences). Implement this
validation in a constructor or TryFrom/validate method that returns Result and
use the parsed Url or a canonicalized repo string inside the struct, returning
errors on validation failure.
d61159e to
ba51002
Compare
2 participants
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.