update readme & configs #59
Conversation
WalkthroughThe pull request introduces updates to the documentation and configuration settings across multiple components related to the OPinit bots. Key changes include incrementing version requirements for various components, adjusting starting heights for L1 and L2 nodes, and refining the query structures for challenges and withdrawals. Additionally, minor formatting corrections and logging adjustments are made to improve clarity and maintain consistency across the codebase. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🔇 Additional comments (7)challenger/query.go (1)
The change from Also applies to: 32-33 challenger/README.md (3)
The change from 0 to 1 for both L1 and L2 start heights is more appropriate as blockchain heights typically start at 1. Also applies to: 49-49
The documentation now clearly specifies:
The challenger/challenger.go (3)
The initialization logic correctly subtracts 1 from start heights to ensure proper synchronization between nodes. Also applies to: 106-106, 114-114
The change from Also applies to: 206-206 Line range hint The logic for calculating start heights has been significantly updated. Please ensure:
✅ Verification successfulLet me analyze the code further with additional scripts to verify the edge cases. Start height calculations are properly handled with appropriate edge cases After analyzing the code and its usage patterns, the start height calculations are correctly implemented:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the usage of start heights in the codebase
# Test: Search for references to start heights to ensure consistent usage
echo "Checking L1/L2 start height references:"
rg -A 3 "l[12]_start_height|L[12]StartHeight"
echo "Checking height initialization patterns:"
ast-grep --pattern 'Initialize($_, $height, $_)'
Length of output: 7643 Script: #!/bin/bash
# Check the logic around l2StartHeight and related conditions
# Check how l2StartHeight is used in conditions
echo "Checking l2StartHeight conditions:"
rg -A 5 "if.*l2StartHeight|if.*l2_start_height"
# Check output handling logic
echo "Checking output handling patterns:"
rg -A 10 "QueryLastFinalizedOutput"
# Check deposit height calculations
echo "Checking deposit height logic:"
rg -A 5 "QueryDepositTxHeight"
Length of output: 3877 🪧 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: 0
🧹 Outside diff range and nitpick comments (2)
challenger/challenger.go (1)
Line range hint
230-287: Consider improving error handling and documentation.The method has complex conditional logic for determining start heights. Consider:
- Adding documentation to explain the logic flow
- Adding error checks for negative height values
- Adding logging statements for better debugging
func (c *Challenger) getNodeStartHeights(ctx types.Context, bridgeId uint64) (l1StartHeight int64, l2StartHeight int64, startOutputIndex uint64, err error) { + // Skip if nodes are already synced if c.host.Node().GetSyncedHeight() != 0 && c.child.Node().GetSyncedHeight() != 0 { return 0, 0, 0, nil } var outputL1Height, outputL2Height int64 var outputIndex uint64 + // Query last finalized output if L2 start height is specified if c.cfg.L2StartHeight > 1 { output, err := c.host.QueryLastFinalizedOutput(ctx, bridgeId) if err != nil { return 0, 0, 0, err } else if output != nil { outputL1Height = types.MustUint64ToInt64(output.OutputProposal.L1BlockNumber) outputL2Height = types.MustUint64ToInt64(output.OutputProposal.L2BlockNumber) outputIndex = output.OutputIndex + ctx.Logger().Debug("Found last finalized output", + zap.Int64("l1_height", outputL1Height), + zap.Int64("l2_height", outputL2Height), + zap.Uint64("output_index", outputIndex)) } }executor/README.md (1)
232-247: Consider improving struct field documentation.The QueryWithdrawalResponse struct fields are now better organized with logical grouping, but could benefit from field-level documentation.
type QueryWithdrawalResponse struct { // fields required to withdraw funds Sequence uint64 `json:"sequence"` - To string `json:"to"` + To string `json:"to"` // destination address for withdrawal - From string `json:"from"` + From string `json:"from"` // source address of withdrawal Amount types.Coin `json:"amount"` OutputIndex uint64 `json:"output_index"` BridgeId uint64 `json:"bridge_id"` WithdrawalProofs [][]byte `json:"withdrawal_proofs"` Version []byte `json:"version"` StorageRoot []byte `json:"storage_root"` LastBlockHash []byte `json:"last_block_hash"` // extra info TxTime int64 `json:"tx_time"` TxHeight int64 `json:"tx_height"` TxHash string `json:"tx_hash"` }🧰 Tools
🪛 Markdownlint (0.35.0)
233-233: Column: 1
Hard tabs(MD010, no-hard-tabs)
234-234: Column: 1
Hard tabs(MD010, no-hard-tabs)
235-235: Column: 1
Hard tabs(MD010, no-hard-tabs)
236-236: Column: 1
Hard tabs(MD010, no-hard-tabs)
237-237: Column: 1
Hard tabs(MD010, no-hard-tabs)
238-238: Column: 1
Hard tabs(MD010, no-hard-tabs)
239-239: Column: 1
Hard tabs(MD010, no-hard-tabs)
240-240: Column: 1
Hard tabs(MD010, no-hard-tabs)
241-241: Column: 1
Hard tabs(MD010, no-hard-tabs)
242-242: Column: 1
Hard tabs(MD010, no-hard-tabs)
244-244: Column: 1
Hard tabs(MD010, no-hard-tabs)
245-245: Column: 1
Hard tabs(MD010, no-hard-tabs)
246-246: Column: 1
Hard tabs(MD010, no-hard-tabs)
247-247: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
README.md(2 hunks)challenger/README.md(2 hunks)challenger/challenger.go(3 hunks)challenger/types/config.go(2 hunks)executor/README.md(5 hunks)node/process.go(0 hunks)server/server.go(1 hunks)
💤 Files with no reviewable changes (1)
- node/process.go
✅ Files skipped from review due to trivial changes (1)
- server/server.go
🧰 Additional context used
🪛 Markdownlint (0.35.0)
executor/README.md
233-233: Column: 1
Hard tabs
(MD010, no-hard-tabs)
234-234: Column: 1
Hard tabs
(MD010, no-hard-tabs)
235-235: Column: 1
Hard tabs
(MD010, no-hard-tabs)
236-236: Column: 1
Hard tabs
(MD010, no-hard-tabs)
237-237: Column: 1
Hard tabs
(MD010, no-hard-tabs)
238-238: Column: 1
Hard tabs
(MD010, no-hard-tabs)
239-239: Column: 1
Hard tabs
(MD010, no-hard-tabs)
240-240: Column: 1
Hard tabs
(MD010, no-hard-tabs)
241-241: Column: 1
Hard tabs
(MD010, no-hard-tabs)
242-242: Column: 1
Hard tabs
(MD010, no-hard-tabs)
244-244: Column: 1
Hard tabs
(MD010, no-hard-tabs)
245-245: Column: 1
Hard tabs
(MD010, no-hard-tabs)
246-246: Column: 1
Hard tabs
(MD010, no-hard-tabs)
247-247: Column: 1
Hard tabs
(MD010, no-hard-tabs)
🔇 Additional comments (9)
README.md (2)
88-88: LGTM: Formatting improvement
The removal of extra space before the comma improves readability.
22-22: Verify compatibility with updated version requirements
The version requirements for all components have been increased:
- L1 Node: v0.6.1+ → v0.6.4+
- MiniMove: v0.6.4+ → v0.6.5+
- MiniWasm: v0.6.4+ → v0.6.5+
- MiniEVM: v0.6.6+ → v0.6.7+
✅ Verification successful
Based on the search results, I can now generate the final response:
Version requirements are consistent with dependencies
The version requirements in the table match the actual dependencies in go.mod:
- OPinit v0.6.1 is listed as a direct dependency
- OPinit/api v0.6.0 is listed as an indirect dependency
These versions are compatible with the updated version requirements table, and there's no evidence of incompatibility issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these versions exist in the repositories
for repo in l1-node mini-move mini-wasm mini-evm; do
echo "Checking $repo tags..."
gh api repos/initia-labs/$repo/tags --jq '.[].name'
done
Length of output: 1266
Script:
#!/bin/bash
# Let's check the git history of README.md to understand the version changes
git log -p README.md | grep -A 5 -B 5 "v0.6"
# Also check if there are any other files mentioning version requirements
rg "v0\.6\.[0-9]+" -A 2 -B 2
Length of output: 20551
challenger/types/config.go (2)
104-109: Validation logic strengthened for start heights
The validation now ensures start heights are strictly greater than 0 (changed from >= 0 to > 0). This aligns with the new default values.
78-79:
Breaking change: Default start heights updated
The default start heights for both L1 and L2 have been changed from 0 to 1. This is a breaking change that could affect existing configurations.
challenger/README.md (2)
44-49: Config example updated to reflect new defaults
The example configuration has been updated to use start height of 1, which aligns with the new defaults in the code.
187-203: Documentation improved with pagination details
The challenges query endpoint has been simplified and enhanced with:
- Removal of the
{page}parameter - Clear documentation of default options
- New
QueryChallengesResponsestructure for pagination
challenger/challenger.go (1)
100-114: Verify the off-by-one adjustments in node initialization.
The initialization of both host and child nodes subtracts 1 from their respective start heights. While this appears intentional, we should verify this behavior is correct across the codebase.
✅ Verification successful
The previous pattern was too specific. Let's try a broader search to understand the initialization patterns in the codebase.
Based on the search results, I can now generate a final response as we have sufficient evidence to verify the off-by-one adjustments in node initialization.
The off-by-one adjustments in node initialization are consistent and correct across the codebase.
The pattern of subtracting 1 from start heights is consistently applied across different components:
- In challenger/challenger.go: Both host and child nodes use
height-1 - In executor/executor.go: All three components (host, child, and batch submitter) use
height-1 - This adjustment ensures initialization starts from the last fully processed height, allowing the system to resume from the next unprocessed height, which is a common pattern in blockchain systems to maintain consistency and avoid gaps in processing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other node initialization patterns to verify consistency
ast-grep --pattern 'Initialize(ctx, $height, $_, $_, $_)'
Length of output: 59
Script:
#!/bin/bash
# Search for Initialize method definitions and usages with a broader pattern
ast-grep --pattern 'Initialize($$$)'
# Also search using ripgrep for Initialize calls and definitions
rg "Initialize.*\(" -A 5
Length of output: 17296
executor/README.md (2)
81-89: Documentation update for start height configurations looks good.
The changes from 0 to 1 for start heights are well documented and consistent with the implementation changes.
418-418: Documentation for pagination is clear and helpful.
The addition of pagination information and the note about using next as an offset is well documented.
Also applies to: 456-458
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
Chores