feat: support staking btc by bnb chain #71
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent updates enhance the BNB light client module by upgrading the Go version across CI configurations and introducing new functionalities for managing blockchain headers and transactions. Key improvements include robust keeper methods, enhanced error handling, and comprehensive protocol buffer definitions. These changes ensure better compatibility with the latest Go features while expanding the overall capabilities of the project. Changes
PoemThank 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 as PR comments)
Additionally, you can add 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
Files selected for processing (6)
- proto/lorenzo/bnblightclient/v1/client.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/genesis.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/params.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/query.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/tx.proto (1 hunks)
- proto/lorenzo/btcstaking/v1/tx.proto (2 hunks)
Additional comments not posted (6)
proto/lorenzo/bnblightclient/v1/params.proto (1)
1-18: Review ofparams.proto: Well-defined structure and clear parameter definitions.
- Syntax and Package: The syntax is correctly set to proto3, and the package name is appropriately scoped.
- Imports and Options: The import of
gogoproto/gogo.protoand the go package option are correctly specified.- Message Definition: The
Paramsmessage is well-defined with clear comments explaining each field. The use ofgogoproto.equalensures that the equality method is generated, which is useful for parameter comparison.Overall, the file is well-structured and serves its purpose effectively.
proto/lorenzo/bnblightclient/v1/genesis.proto (1)
1-17: Review ofgenesis.proto: Well-structured and comprehensive definition of genesis state.
- Imports and Options: Correctly imports necessary proto files and sets the go package option.
- GenesisState Message: The
GenesisStatemessage effectively links theParamsandHeadermessages, providing a comprehensive setup for the genesis state. The inclusion oftx_hashesis a good practice for tracking transactions.This file is correctly set up to initialize the BNB light client's genesis state.
proto/lorenzo/bnblightclient/v1/query.proto (2)
11-28: Review of gRPC Query ServiceThe service definition is well-structured with clear RPC methods for fetching parameters, specific headers, and the latest header. The use of HTTP GET methods in the annotations is appropriate for these query operations.
30-57: Review of Message DefinitionsThe message definitions are consistent and well-defined. Each request and response type is clearly associated with its corresponding RPC method. The use of
(gogoproto.nullable) = falseon critical fields ensures that these fields must be provided, enhancing the robustness of the interface.proto/lorenzo/btcstaking/v1/tx.proto (2)
18-20: Review of New RPC Method for BTC StakingThe addition of
CreateBTCStakingFromBNBis aligned with the PR's objective to support BTC staking by BNB chain. The method signature is clear, and the response type is appropriately defined.
64-73: Review of Message Types for BTC StakingThe new message types
MsgCreateBTCStakingFromBNBandMsgCreateBTCStakingFromBNBResponseare well-defined. The fieldssigner,transaction,receipt, andproofinMsgCreateBTCStakingFromBNBare essential for the staking operation, ensuring that all necessary data is encapsulated within the message.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- proto/lorenzo/bnblightclient/v1/params.proto (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/lorenzo/bnblightclient/v1/params.proto
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (7)
x/bnblightclient/types/client.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/genesis.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/params.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gw.gois excluded by!**/*.pb.gw.gox/bnblightclient/types/tx.pb.gois excluded by!**/*.pb.gox/btcstaking/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (3)
- proto/lorenzo/bnblightclient/v1/client.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/tx.proto (1 hunks)
- proto/lorenzo/btcstaking/v1/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- proto/lorenzo/bnblightclient/v1/client.proto
- proto/lorenzo/bnblightclient/v1/tx.proto
- proto/lorenzo/btcstaking/v1/tx.proto
There was a problem hiding this comment.
Actionable comments posted: 8
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- x/bnblightclient/keeper/grpc_query.go (1 hunks)
- x/bnblightclient/keeper/keeper.go (1 hunks)
- x/bnblightclient/keeper/msg_server.go (1 hunks)
- x/btcstaking/keeper/msg_server.go (2 hunks)
Additional context used
golangci-lint
x/bnblightclient/keeper/keeper.go
5-5: File is not
gofumpt-ed(gofumpt)
x/bnblightclient/keeper/grpc_query.go
13-13: field
kis unused(unused)
x/bnblightclient/keeper/msg_server.go
35-35: File is not
goimports-ed(goimports)
36-36: File is not
gofumpt-ed(gofumpt)
x/btcstaking/keeper/msg_server.go
34-34: File is not
goimports-ed(goimports)
GitHub Check: golangci-lint
x/bnblightclient/keeper/grpc_query.go
[failure] 13-13:
fieldkis unused (unused)
x/btcstaking/keeper/msg_server.go
Outdated
| // CreateBTCStakingFromBNB implements types.MsgServer. | ||
| func (ms msgServer) CreateBTCStakingFromBNB(context.Context, *types.MsgCreateBTCStakingFromBNB) (*types.MsgCreateBTCStakingFromBNBResponse, error) { | ||
| panic("unimplemented") | ||
| } |
There was a problem hiding this comment.
Implement the CreateBTCStakingFromBNB function.
The CreateBTCStakingFromBNB function is currently unimplemented. Ensure to provide the necessary implementation.
Do you need assistance in implementing the CreateBTCStakingFromBNB function?
x/btcstaking/keeper/msg_server.go
Outdated
| @@ -31,6 +31,8 @@ | |||
| *Keeper | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Format the file using goimports.
The file is not formatted according to goimports standards.
package keeper
import (
"bytes"
"context"
"encoding/hex"
"fmt"
errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/Lorenzo-Protocol/lorenzo/x/btcstaking/types"
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
"github.com/ethereum/go-ethereum/common"
)
const EthAddrLen = 42
const (
Dep0Amount = 4e5
Dep1Amount = 2e6
Dep2Amount = 1e7
Dep3Amount = 5e7
)
type msgServer struct {
*Keeper
}
var _ types.MsgServer = msgServer{}
func NewMsgServerImpl(keeper *Keeper) types.MsgServer {
return &msgServer{Keeper: keeper}
}
func NewBTCTxFromBytes(txBytes []byte) (*wire.MsgTx, error) {
var msgTx wire.MsgTx
rbuf := bytes.NewReader(txBytes)
if err := msgTx.Deserialize(rbuf); err != nil {
return nil, err
}
return &msgTx, nil
}
const maxOpReturnPkScriptSize = 83
func ExtractPaymentTo(tx *wire.MsgTx, addr btcutil.Address) (uint64, error) {
payToAddrScript, err := txscript.PayToAddrScript(addr)
if err != nil {
return 0, fmt.Errorf("invalid address")
}
var amt uint64 = 0
for _, out := range tx.TxOut {
if bytes.Equal(out.PkScript, payToAddrScript) {
amt += uint64(out.Value)
}
}
return amt, nil
}
func ExtractPaymentToWithOpReturnIdAndDust(tx *wire.MsgTx, addr btcutil.Address, dustAmount int64) (uint64, []byte, error) {
payToAddrScript, err := txscript.PayToAddrScript(addr)
if err != nil {
return 0, nil, fmt.Errorf("invalid address")
}
var amt uint64 = 0
foundOpReturnId := false
var opReturnId []byte
for _, out := range tx.TxOut {
if bytes.Equal(out.PkScript, payToAddrScript) && out.Value >= dustAmount {
amt += uint64(out.Value)
} else {
pkScript := out.PkScript
pkScriptLen := len(pkScript)
// valid op return script will have at least 2 bytes
// - fisrt byte should be OP_RETURN marker
// - second byte should indicate how many bytes there are in opreturn script
if pkScriptLen > 1 &&
pkScriptLen <= maxOpReturnPkScriptSize &&
pkScript[0] == txscript.OP_RETURN {
// if this is OP_PUSHDATA1, we need to drop first 3 bytes as those are related
// to script iteslf i.e OP_RETURN + OP_PUSHDATA1 + len of bytes
if pkScript[1] == txscript.OP_PUSHDATA1 {
opReturnId = pkScript[3:]
} else if pkScript[1] == txscript.OP_PUSHDATA2 {
opReturnId = pkScript[4:]
} else if pkScript[1] == txscript.OP_PUSHDATA4 {
opReturnId = pkScript[6:]
} else {
// this should be one of OP_DATAXX opcodes we drop first 2 bytes
opReturnId = pkScript[2:]
}
foundOpReturnId = true
}
}
}
if !foundOpReturnId {
return 0, nil, fmt.Errorf("expected op_return_id not found")
}
return amt, opReturnId, nil
}
func canPerformMint(signer sdk.AccAddress, p types.Params) bool {
if len(p.MinterAllowList) == 0 {
return true
}
for _, addr := range p.MinterAllowList {
if sdk.MustAccAddressFromBech32(addr).Equals(signer) {
return true
}
}
return false
}
func (ms msgServer) CreateBTCStaking(goCtx context.Context, req *types.MsgCreateBTCStaking) (*types.MsgCreateBTCStakingResponse, error) {
stakingMsgTx, err := NewBTCTxFromBytes(req.StakingTx.Transaction)
if err != nil {
return nil, types.ErrParseBTCTx.Wrap(err.Error())
}
ctx := sdk.UnwrapSDKContext(goCtx)
stakingTxHash := stakingMsgTx.TxHash()
stakingRecord := ms.getBTCStakingRecord(ctx, stakingTxHash)
if stakingRecord != nil {
return nil, types.ErrDupBTCTx
}
stakingTxHeader := ms.btclcKeeper.GetHeaderByHash(ctx, req.StakingTx.Key.Hash)
if stakingTxHeader == nil {
return nil, types.ErrBlkHdrNotFound
}
p := ms.GetParams(ctx)
btcTip := ms.btclcKeeper.GetTipInfo(ctx)
stakingTxDepth := btcTip.Height - stakingTxHeader.Height
btclcParams := ms.btclcKeeper.GetBTCNet()
if err := req.StakingTx.VerifyInclusion(stakingTxHeader.Header, btclcParams.PowLimit); err != nil {
return nil, types.ErrBTCTxNotIncluded.Wrap(err.Error())
}
_, receiver := findReceiver(p.Receivers, req.Receiver)
if receiver == nil {
return nil, types.ErrInvalidReceivingAddr.Wrapf("Receiver(%s) not exists", req.Receiver)
}
btcReceivingAddr, err := btcutil.DecodeAddress(receiver.Addr, btclcParams)
if err != nil {
return nil, types.ErrInvalidReceivingAddr.Wrap(err.Error())
}
var mintToAddr []byte
var btcAmount uint64
if common.IsHexAddress(receiver.EthAddr) {
signers := req.GetSigners()
if len(signers) == 0 || !canPerformMint(req.GetSigners()[0], *p) {
return nil, types.ErrNotInAllowList
}
mintToAddr = common.HexToAddress(receiver.EthAddr).Bytes()
btcAmount, err = ExtractPaymentTo(stakingMsgTx, btcReceivingAddr)
} else {
btcAmount, mintToAddr, err = ExtractPaymentToWithOpReturnIdAndDust(stakingMsgTx, btcReceivingAddr, p.TxoutDustAmount)
}
if err != nil || btcAmount == 0 {
return nil, types.ErrInvalidTransaction
} else if btcAmount < Dep0Amount { // no depth check required
} else if btcAmount < Dep1Amount { // at least 1 depth
if stakingTxDepth < 1 {
return nil, types.ErrBlkHdrNotConfirmed.Wrapf("not k-deep: k=1; depth=%d", stakingTxDepth)
}
} else if btcAmount < Dep2Amount {
if stakingTxDepth < 2 {
return nil, types.ErrBlkHdrNotConfirmed.Wrapf("not k-deep: k=2; depth=%d", stakingTxDepth)
}
} else if btcAmount < Dep3Amount {
if stakingTxDepth < 3 {
return nil, types.ErrBlkHdrNotConfirmed.Wrapf("not k-deep: k=3; depth=%d", stakingTxDepth)
}
} else if stakingTxDepth < 4 {
return nil, types.ErrBlkHdrNotConfirmed.Wrapf("not k-deep: k=4; depth=%d", stakingTxDepth)
}
if len(mintToAddr) != 20 {
return nil, types.ErrMintToAddr.Wrap(hex.EncodeToString(mintToAddr))
}
toMintAmount := sdkmath.NewIntFromUint64(btcAmount).Mul(sdkmath.NewIntFromUint64(1e10))
coins := []sdk.Coin{
{
Denom: types.NativeTokenDenom,
Amount: toMintAmount,
},
}
err = ms.bankKeeper.MintCoins(ctx, types.ModuleName, coins)
if err != nil {
return nil, types.ErrMintToModule.Wrap(err.Error())
}
err = ms.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, mintToAddr, coins)
if err != nil {
return nil, types.ErrTransferToAddr.Wrap(err.Error())
}
bctStakingRecord := types.BTCStakingRecord{
TxHash: stakingTxHash[:],
Amount: btcAmount,
MintToAddr: mintToAddr,
BtcReceiverName: receiver.Name,
BtcReceiverAddr: receiver.Addr,
}
err = ms.addBTCStakingRecord(ctx, &bctStakingRecord)
if err != nil {
return nil, types.ErrRecordStaking.Wrap(err.Error())
}
err = ctx.EventManager().EmitTypedEvent(types.NewEventBTCStakingCreated(&bctStakingRecord))
if err != nil {
panic(fmt.Errorf("fail to emit EventBTCStakingCreated : %w", err))
}
return &types.MsgCreateBTCStakingResponse{}, nil
}
func (ms msgServer) Burn(goCtx context.Context, req *types.MsgBurnRequest) (*types.MsgBurnResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
btcNetworkParams := ms.btclcKeeper.GetBTCNet()
btcTargetAddress, err := btcutil.DecodeAddress(req.BtcTargetAddress, btcNetworkParams)
if err != nil {
return nil, types.ErrInvalidBurnBtcTargetAddress.Wrap(err.Error())
}
amount := sdk.NewCoin(types.NativeTokenDenom, req.Amount)
signers := req.GetSigners()
if len(signers) != 1 {
return nil, types.ErrBurnInvalidSigner
}
signer := signers[0]
balance := ms.bankKeeper.GetBalance(ctx, signer, types.NativeTokenDenom)
if balance.IsLT(amount) {
return nil, types.ErrBurnInsufficientBalance
}
err = ms.bankKeeper.SendCoinsFromAccountToModule(ctx, signer, types.ModuleName, []sdk.Coin{amount})
if err != nil {
return nil, types.ErrBurn.Wrap(err.Error())
}
err = ms.bankKeeper.BurnCoins(ctx, types.ModuleName, []sdk.Coin{amount})
if err != nil {
return nil, types.ErrBurn.Wrap(err.Error())
}
err = ctx.EventManager().EmitTypedEvent(types.NewEventBurnCreated(signer, btcTargetAddress, amount))
if err != nil {
return nil, types.ErrEmitEvent.Wrap(err.Error())
}
return &types.MsgBurnResponse{}, nil
}
// CreateBTCStakingFromBNB implements types.MsgServer.
func (ms msgServer) CreateBTCStakingFromBNB(context.Context, *types.MsgCreateBTCStakingFromBNB) (*types.MsgCreateBTCStakingFromBNBResponse, error) {
panic("unimplemented")
}
func findReceiver(receivers []*types.Receiver, name string) (int, *types.Receiver) {
var receiver *types.Receiver = nil
idx := -1
for i, r := range receivers {
if r != nil && r.Name == name {
idx = i
receiver = r
break
}
}
return idx, receiver
}
func (ms msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
if ms.authority != req.Authority {
return nil, errorsmod.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", ms.authority, req.Authority)
}
ctx := sdk.UnwrapSDKContext(goCtx)
if err := req.Params.Validate(); err != nil {
return nil, err
}
if err := ms.SetParams(ctx, &req.Params); err != nil {
return nil, err
}
return &types.MsgUpdateParamsResponse{}, nil
}
func (ms msgServer) AddReceiver(goCtx context.Context, req *types.MsgAddReceiver) (*types.MsgAddReceiverResponse, error) {
return nil, fmt.Errorf("deprecated, use UpdateParams instead")
}
func (ms msgServer) RemoveReceiver(goCtx context.Context, req *types.MsgRemoveReceiver) (*types.MsgRemoveReceiverResponse, error) {
return nil, fmt.Errorf("deprecated, use UpdateParams instead")
}
Committable suggestion was skipped due to low confidence.
Tools
golangci-lint
34-34: File is not
goimports-ed(goimports)
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (3)
x/bnblightclient/keeper/keeper.go (1)
51-52: Consider logging errors for better debugging.Logging errors can help in debugging issues with proof verification.
- return errorsmod.Wrapf(types.ErrInvalidProof, "invalid receipt proof") + // Log the error before returning + ctx.Logger().Error("invalid receipt proof", "error", err) + return errorsmod.Wrapf(types.ErrInvalidProof, "invalid receipt proof")x/bnblightclient/keeper/header.go (2)
25-27: Consider logging errors for better debugging.Logging errors can help in debugging issues with header verification.
- return err + // Log the error before returning + ctx.Logger().Error("failed to verify headers", "error", err) + return err
45-47: Consider logging errors for better debugging.Logging errors can help in debugging issues with header updates.
- return err + // Log the error before returning + ctx.Logger().Error("failed to verify header", "error", err) + return err
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
x/bnblightclient/types/client.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/params.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gw.gois excluded by!**/*.pb.gw.gox/bnblightclient/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (15)
- go.mod (2 hunks)
- proto/lorenzo/bnblightclient/v1/client.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/params.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/query.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/tx.proto (1 hunks)
- x/bnblightclient/keeper/grpc_query.go (1 hunks)
- x/bnblightclient/keeper/header.go (1 hunks)
- x/bnblightclient/keeper/keeper.go (1 hunks)
- x/bnblightclient/keeper/msg_server.go (1 hunks)
- x/bnblightclient/keeper/params.go (1 hunks)
- x/bnblightclient/types/errors.go (1 hunks)
- x/bnblightclient/types/header.go (1 hunks)
- x/bnblightclient/types/keys.go (1 hunks)
- x/bnblightclient/types/params.go (1 hunks)
- x/bnblightclient/types/proof.go (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- proto/lorenzo/bnblightclient/v1/client.proto
- proto/lorenzo/bnblightclient/v1/params.proto
- proto/lorenzo/bnblightclient/v1/query.proto
- proto/lorenzo/bnblightclient/v1/tx.proto
- x/bnblightclient/keeper/grpc_query.go
- x/bnblightclient/keeper/msg_server.go
Additional context used
golangci-lint
x/bnblightclient/types/proof.go
127-127: unnecessary conversion
(unconvert)
x/bnblightclient/types/header.go
64-64: File is not
gofumpt-ed(gofumpt)
77-77: File is not
gofumpt-ed(gofumpt)
83-83: File is not
gofumpt-ed(gofumpt)
152-152: Error return value is not checked
(errcheck)
155-155: Error return value of
rlp.Encodeis not checked(errcheck)
156-156: Error return value of
sha.Readis not checked(errcheck)
GitHub Check: test-unit-cover
x/bnblightclient/keeper/header.go
[failure] 4-4:
package slices is not in GOROOT (/opt/hostedtoolcache/go/1.20.14/x64/src/slices)
Additional comments not posted (26)
x/bnblightclient/keeper/params.go (2)
14-22: LGTM!The
SetParamsfunction correctly sets the parameters in the key-value store after validating them.
29-36: LGTM!The
GetParamsfunction correctly retrieves the parameters from the key-value store and unmarshals them.x/bnblightclient/types/params.go (2)
14-30: LGTM!The
Validatemethod correctly validates theParamsstruct and returns appropriate error messages.
36-50: LGTM!The
ValidateAllowListfunction correctly validates the list of addresses and returns appropriate error messages.x/bnblightclient/types/keys.go (4)
8-14: LGTM!The constants are defined correctly and follow a consistent naming convention.
33-36: LGTM!The
KeyHeaderfunction correctly generates the key for a header using the block number.
39-41: LGTM!The
KeyHeaderHashfunction correctly generates the key for the header hash.
44-46: LGTM!The
KeyLatestedHeaderNumberfunction correctly returns the key for the latest header number.x/bnblightclient/keeper/keeper.go (1)
19-25: LGTM! TheKeeperstruct is well-defined.The
Keeperstruct now has meaningful fields for store interaction, codec, and authority management.x/bnblightclient/types/proof.go (8)
14-19: LGTM! TheProofstruct is well-defined.The
Proofstruct contains fieldsIndex,Value, andPath, with appropriate JSON tags.
21-25: LGTM! TheProofPathstruct is well-defined.The
ProofPathstruct contains fieldsKeysandValues, with appropriate JSON tags.
35-50: LGTM! ThePutmethod is well-structured.The
Putmethod adds a new key-value pair to theProofPathand handles errors appropriately.
52-67: LGTM! TheDeletemethod is well-structured.The
Deletemethod removes a key-value pair from theProofPathand handles errors appropriately.
69-79: LGTM! TheHasmethod is well-structured.The
Hasmethod checks if a key exists in theProofPathand handles errors appropriately.
81-92: LGTM! TheGetmethod is well-structured.The
Getmethod returns the value of a key in theProofPathand handles errors appropriately.
94-96: LGTM! TheencodeKeymethod is well-structured.The
encodeKeymethod encodes a key using base64 encoding.
98-105: LGTM! ThevalueIdxmethod is well-structured.The
valueIdxmethod returns the index of a key in theProofPath.x/bnblightclient/keeper/header.go (7)
58-75: LGTM! TheGetLatestedHeadermethod is well-structured.The
GetLatestedHeadermethod retrieves the latest header from the store and handles errors appropriately.
77-95: LGTM! TheGetHeadermethod is well-structured.The
GetHeadermethod retrieves the header for a specific number from the store and handles errors appropriately.
97-115: LGTM! TheGetHeaderByHashmethod is well-structured.The
GetHeaderByHashmethod retrieves a header from the store based on its hash and handles errors appropriately.
117-126: LGTM! TheHasHeadermethod is well-structured.The
HasHeadermethod checks if a header with the given number exists in the store and handles errors appropriately.
128-135: LGTM! ThesetHeadermethod is well-structured.The
setHeadermethod sets a header in the store and handles errors appropriately.
137-141: LGTM! ThesetLatestedHeaderNumbermethod is well-structured.The
setLatestedHeaderNumbermethod sets the latest header number in the store and handles errors appropriately.
14-16: Handle potential errors when sorting headers.The method should handle potential errors when sorting headers.
- slices.SortFunc(headers, func(a, b *types.Header) int { - return int(a.Number - b.Number) - }) + sort.SliceStable(headers, func(i, j int) bool { + return headers[i].Number < headers[j].Number + })Likely invalid or redundant comment.
x/bnblightclient/types/header.go (1)
24-56: LGTM! TheBNBHeaderstruct is well-defined.The
BNBHeaderstruct represents a block header in the Ethereum blockchain and uses appropriate JSON and RLP tags.go.mod (1)
43-43: Change approved: Movedgolang.org/x/cryptoto the primary require block.The change ensures that
golang.org/x/cryptois included in the primary set of dependencies, which might be necessary for compatibility or build reasons.However, ensure that the dependency is used correctly throughout the codebase.
Verification successful
Verified: The
golang.org/x/cryptodependency is used correctly in the codebase.The import and usage of
golang.org/x/crypto/sha3inx/bnblightclient/types/header.goare appropriate and align with the intended functionality. The dependency is correctly integrated and should be compatible with the specified versionv0.21.0.
x/bnblightclient/types/header.go: Usage ofsha3.NewLegacyKeccak256()within async.Poolis valid.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `golang.org/x/crypto` in the codebase. # Test: Search for the usage of `golang.org/x/crypto`. Expect: Valid usages. rg --type go 'golang.org/x/crypto'Length of output: 95
Script:
#!/bin/bash # Description: Extract and review the relevant lines from `x/bnblightclient/types/header.go` for the usage of `golang.org/x/crypto`. # Extract lines containing the usage of `golang.org/x/crypto` and a few lines of context. rg --type go --context 5 'golang.org/x/crypto/sha3' x/bnblightclient/types/header.goLength of output: 376
Script:
#!/bin/bash # Description: Extract and review the entire file `x/bnblightclient/types/header.go` to locate the actual usage of `golang.org/x/crypto/sha3`. # Extract the entire content of the file for review. cat x/bnblightclient/types/header.goLength of output: 6060
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/bnblightclient/types/genesis.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/params.pb.gois excluded by!**/*.pb.go
Files selected for processing (14)
- .github/workflows/codeql.yml (1 hunks)
- .github/workflows/golangci_lint.yml (1 hunks)
- .github/workflows/release_binary.yml (1 hunks)
- .github/workflows/test.yml (1 hunks)
- go.mod (3 hunks)
- proto/lorenzo/bnblightclient/v1/genesis.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/params.proto (1 hunks)
- x/bnblightclient/keeper/keeper.go (1 hunks)
- x/bnblightclient/keeper/proof.go (1 hunks)
- x/bnblightclient/types/errors.go (1 hunks)
- x/bnblightclient/types/events.go (1 hunks)
- x/bnblightclient/types/keys.go (1 hunks)
- x/bnblightclient/types/params.go (1 hunks)
- x/bnblightclient/types/proof.go (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/codeql.yml
- .github/workflows/release_binary.yml
Files skipped from review as they are similar to previous changes (8)
- go.mod
- proto/lorenzo/bnblightclient/v1/genesis.proto
- proto/lorenzo/bnblightclient/v1/params.proto
- x/bnblightclient/keeper/keeper.go
- x/bnblightclient/types/errors.go
- x/bnblightclient/types/keys.go
- x/bnblightclient/types/params.go
- x/bnblightclient/types/proof.go
Additional comments not posted (36)
.github/workflows/golangci_lint.yml (1)
18-18: LGTM! But verify compatibility with Go 1.21.The update to Go 1.21 is approved.
However, ensure that the rest of the codebase is compatible with Go 1.21.
x/bnblightclient/types/events.go (1)
13-21: LGTM! Ensure completeness of event handling logic.The struct definition for
BNBCrossChainEventand theKeyfunction are approved.However, verify that the event handling logic and ABI are complete and accurate.
.github/workflows/test.yml (1)
23-23: LGTM! But verify compatibility with Go 1.21.The update to Go 1.21 is approved.
However, ensure that the rest of the codebase is compatible with Go 1.21.
x/bnblightclient/keeper/proof.go (33)
24-31: Ensure proper error handling forverifyProof.The function
verifyProofis called, and if it returns an error, the function returnsniland the error. This is a good practice for error handling.
33-36: Ensure proper error handling forparseEvents.The function
parseEventsis called, and if it returns an error, the function returnsniland the error. This is a good practice for error handling.
38-40: Handle case when no events are found.The function returns an error if no events are found in the receipt. This ensures that invalid receipts are not processed further.
41-41: Return parsed events.The function returns the parsed events if no errors are encountered. This is the expected behavior.
48-49: Retrieve the key-value store.The function retrieves the key-value store from the context using the store key. This is a standard practice.
51-52: Ensure proper iterator closure.The iterator is closed using
defer. This ensures that the iterator is properly closed even if an error occurs.
54-58: Iterate over the key-value store and unmarshal event records.The function iterates over the key-value store and unmarshals each event record. This is a standard practice for retrieving and processing data from a key-value store.
59-59: Return all retrieved event records.The function returns all the retrieved event records. This is the expected behavior.
63-64: Check receipt status.The function checks if the receipt status is successful. This ensures that only successful transactions are verified.
67-69: Create trie database and derive SHA.The function creates a trie database and derives the SHA for the receipt. This is a standard practice for verifying proofs.
71-73: Get transaction value from trie.The function gets the transaction value from the trie. This is a standard practice for verifying proofs.
75-77: Check if header exists.The function checks if the header exists for the given block number. If not, it returns an error. This ensures that only valid headers are processed.
80-83: Verify proof.The function verifies the proof using the trie. If the proof is invalid, it returns an error. This ensures that only valid proofs are processed.
85-87: Check proof values.The function checks if the proof values match the expected values. If not, it returns an error. This ensures that only valid proofs are processed.
89-89: Return nil if verification is successful.The function returns nil if the verification is successful. This is the expected behavior.
93-95: Check for logs in the receipt.The function checks if there are logs in the receipt. If not, it returns an error. This ensures that only receipts with logs are processed.
97-98: Retrieve parameters.The function retrieves the parameters from the context. This is a standard practice for accessing configuration data.
100-104: Iterate over logs and filter by contract address.The function iterates over the logs and filters them by the contract address. This ensures that only relevant logs are processed.
106-112: Check the number of topics in the log.The function checks if the number of topics in the log is correct. If not, it returns an error. This ensures that only valid logs are processed.
114-118: Retrieve event by ID and handle errors.The function retrieves the event by its ID and handles any errors that occur. This ensures that only valid events are processed.
120-122: Filter events by name.The function filters the events by their name. This ensures that only relevant events are processed.
124-127: Unpack event data and handle errors.The function unpacks the event data and handles any errors that occur. This ensures that only valid event data is processed.
129-135: Check the number of event parameters.The function checks if the number of event parameters is correct. If not, it returns an error. This ensures that only valid events are processed.
137-145: Create event record and handle duplicates.The function creates an event record and checks if it already exists. If it does, it returns an error. This ensures that duplicate events are not processed.
148-156: Extract and validate event parameters.The function extracts and validates the event parameters. If any parameter is invalid, it returns an error. This ensures that only valid event parameters are processed.
159-165: Extract and validate additional event parameters.The function extracts and validates additional event parameters. If any parameter is invalid, it returns an error. This ensures that only valid event parameters are processed.
167-175: Create and append BNBCrossChainEvent.The function creates a
BNBCrossChainEventand appends it to the list of events. This is the expected behavior.
177-177: Return parsed events.The function returns the parsed events. This is the expected behavior.
181-183: Retrieve the key-value store and check for key existence.The function retrieves the key-value store and checks if the key exists. This is a standard practice for checking the existence of a record.
183-183: Return boolean indicating existence of the record.The function returns a boolean indicating the existence of the record. This is the expected behavior.
187-188: Retrieve the key-value store.The function retrieves the key-value store from the context using the store key. This is a standard practice.
190-191: Marshal the event record.The function marshals the event record using the codec. This is a standard practice for serializing data.
191-191: Set the key-value pair in the store.The function sets the key-value pair in the store. This is the expected behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
x/bnblightclient/keeper/prune.go (1)
8-22: Rename variablelatestedHeadertolatestHeader.The variable name
latestedHeaderis grammatically incorrect. It should be renamed tolatestHeaderfor better readability.- latestedHeader, exist := k.GetLatestedHeader(ctx) + latestHeader, exist := k.GetLatestHeader(ctx) - pruneEndNumber := latestedHeader.Number - params.RetainedBlocks + pruneEndNumber := latestHeader.Number - params.RetainedBlocks
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/bnblightclient/keeper/header.go (1 hunks)
- x/bnblightclient/keeper/prune.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/bnblightclient/keeper/header.go
Additional context used
golangci-lint
x/bnblightclient/keeper/prune.go
27-27: Error return value of
iterator.Closeis not checked(errcheck)
46-46: Error return value of
iterator2.Closeis not checked(errcheck)
GitHub Check: golangci-lint
x/bnblightclient/keeper/prune.go
[failure] 27-27:
Error return value ofiterator.Closeis not checked (errcheck)
[failure] 46-46:
Error return value ofiterator2.Closeis not checked (errcheck)
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/bnblightclient/keeper/header.go (1 hunks)
- x/bnblightclient/keeper/prune.go (1 hunks)
Additional context used
golangci-lint
x/bnblightclient/keeper/prune.go
22-22: Error return value of
iterator.Closeis not checked(errcheck)
41-41: Error return value of
iterator2.Closeis not checked(errcheck)
GitHub Check: golangci-lint
x/bnblightclient/keeper/prune.go
[failure] 22-22:
Error return value ofiterator.Closeis not checked (errcheck)
[failure] 41-41:
Error return value ofiterator2.Closeis not checked (errcheck)
Additional comments not posted (4)
x/bnblightclient/keeper/header.go (4)
45-55: LGTM!The
UpdateHeaderfunction is correctly implemented.
103-112: LGTM!The
GetHeaderfunction is correctly implemented.
123-132: LGTM!The
GetHeaderByHashfunction is correctly implemented.
134-143: LGTM!The
HasHeaderfunction is correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
x/bnblightclient/types/genesis.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/params.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gois excluded by!**/*.pb.gox/bnblightclient/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
Files selected for processing (14)
- proto/lorenzo/bnblightclient/v1/genesis.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/params.proto (1 hunks)
- proto/lorenzo/bnblightclient/v1/query.proto (1 hunks)
- x/bnblightclient/keeper/genesis.go (1 hunks)
- x/bnblightclient/keeper/grpc_query.go (1 hunks)
- x/bnblightclient/keeper/header.go (1 hunks)
- x/bnblightclient/keeper/msg_server.go (1 hunks)
- x/bnblightclient/keeper/params.go (1 hunks)
- x/bnblightclient/keeper/proof.go (1 hunks)
- x/bnblightclient/keeper/prune.go (1 hunks)
- x/bnblightclient/types/genesis.go (1 hunks)
- x/bnblightclient/types/keys.go (1 hunks)
- x/bnblightclient/types/msg.go (1 hunks)
- x/bnblightclient/types/params.go (1 hunks)
Files skipped from review due to trivial changes (2)
- proto/lorenzo/bnblightclient/v1/genesis.proto
- proto/lorenzo/bnblightclient/v1/params.proto
Files skipped from review as they are similar to previous changes (5)
- x/bnblightclient/keeper/grpc_query.go
- x/bnblightclient/keeper/msg_server.go
- x/bnblightclient/keeper/params.go
- x/bnblightclient/keeper/proof.go
- x/bnblightclient/types/keys.go
Additional context used
golangci-lint
x/bnblightclient/keeper/prune.go
22-22: Error return value of
iterator.Closeis not checked(errcheck)
41-41: Error return value of
iterator2.Closeis not checked(errcheck)
x/bnblightclient/types/msg.go
51-51: File is not
goimports-ed(goimports)
GitHub Check: golangci-lint
x/bnblightclient/keeper/prune.go
[failure] 22-22:
Error return value ofiterator.Closeis not checked (errcheck)
[failure] 41-41:
Error return value ofiterator2.Closeis not checked (errcheck)
Additional comments not posted (25)
x/bnblightclient/types/genesis.go (1)
6-11: Verify the implementation ofParams.ValidateandVeryHeaders.The
Validatemethod relies onParams.ValidateandVeryHeadersfor validation. Ensure these methods handle all edge cases and return meaningful error messages.x/bnblightclient/keeper/genesis.go (2)
13-33: Ensure proper error handling and state updates inInitGenesis.The
InitGenesismethod initializes the module's state and handles errors by panicking. Ensure that this is the intended behavior and that all state updates are performed correctly.Verification successful
Verified: Proper error handling and state updates in
InitGenesis.The
InitGenesismethod correctly handles errors usingpanicand performs the necessary state updates through thesetHeader,setLatestNumber, andsetEventRecordfunctions, which correctly interact with the key-value store.
setHeaderimplementation inheader.go(line 166)setLatestNumberimplementation inheader.go(line 175)setEventRecordimplementation inproof.go(line 186)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of error handling and state updates in InitGenesis. # Test: Search for the implementation of setHeader. Expect: Correct state update. rg --type go -A 10 $'func (k Keeper) setHeader(ctx sdk.Context, header types.Header)' # Test: Search for the implementation of setLatestNumber. Expect: Correct state update. rg --type go -A 10 $'func (k Keeper) setLatestNumber(ctx sdk.Context, number uint64)' # Test: Search for the implementation of setEventRecord. Expect: Correct state update. rg --type go -A 10 $'func (k Keeper) setEventRecord(ctx sdk.Context, record types.EventRecord)'Length of output: 270
Script:
#!/bin/bash # Description: Verify the correctness of error handling and state updates in InitGenesis using ast-grep. # Test: Search for the implementation of setHeader. Expect: Correct state update. ast-grep --lang go --pattern $'func (k Keeper) setHeader(ctx $_, header $_) { $$$ }' # Test: Search for the implementation of setLatestNumber. Expect: Correct state update. ast-grep --lang go --pattern $'func (k Keeper) setLatestNumber(ctx $_, number $_) { $$$ }' # Test: Search for the implementation of setEventRecord. Expect: Correct state update. ast-grep --lang go --pattern $'func (k Keeper) setEventRecord(ctx $_, record $_) { $$$ }'Length of output: 1724
35-42: Ensure proper state retrieval inExportGenesis.The
ExportGenesismethod retrieves the current state and returns it as aGenesisState. Ensure that the methodsGetParams,GetAllHeaders, andGetAllEventRecordcorrectly retrieve the state.Verification successful
State retrieval in
ExportGenesisis correct.The methods
GetParams,GetAllHeaders, andGetAllEventRecordcorrectly retrieve the state from the context and return the expected data types. TheExportGenesisfunction inx/bnblightclient/keeper/genesis.goaccurately retrieves the state and returns it as aGenesisState.
GetParamsretrieves parameters from the store.GetAllHeadersretrieves all headers.GetAllEventRecordretrieves all event records.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of state retrieval in ExportGenesis. # Test: Search for the implementation of GetParams. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetParams(ctx sdk.Context) types.Params' # Test: Search for the implementation of GetAllHeaders. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetAllHeaders(ctx sdk.Context) []types.Header' # Test: Search for the implementation of GetAllEventRecord. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetAllEventRecord(ctx sdk.Context) []types.EventRecord'Length of output: 612
Script:
#!/bin/bash # Description: Verify the correctness of state retrieval in ExportGenesis. # Test: Search for the implementation of GetParams. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetParams(ctx sdk.Context) types.Params' # Test: Search for the implementation of GetAllHeaders. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetAllHeaders(ctx sdk.Context) \\[\\]types.Header' # Test: Search for the implementation of GetAllEventRecord. Expect: Correct state retrieval. rg --type go -A 10 $'func (k Keeper) GetAllEventRecord(ctx sdk.Context) \\[\\]types.EventRecord'Length of output: 261
Script:
#!/bin/bash # Description: Verify the correctness of state retrieval in ExportGenesis. # Test: Search for the implementation of GetParams. Expect: Correct state retrieval. rg --type go -A 10 $'func GetParams(ctx sdk.Context) types.Params' # Test: Search for the implementation of GetAllHeaders. Expect: Correct state retrieval. rg --type go -A 10 $'func GetAllHeaders(ctx sdk.Context) \\[\\]types.Header' # Test: Search for the implementation of GetAllEventRecord. Expect: Correct state retrieval. rg --type go -A 10 $'func GetAllEventRecord(ctx sdk.Context) \\[\\]types.EventRecord'Length of output: 228
Script:
#!/bin/bash # Description: Verify the correctness of state retrieval in ExportGenesis. # Test: Search for the implementation of GetParams. Expect: Correct state retrieval. rg --type go -A 10 'GetParams' # Test: Search for the implementation of GetAllHeaders. Expect: Correct state retrieval. rg --type go -A 10 'GetAllHeaders' # Test: Search for the implementation of GetAllEventRecord. Expect: Correct state retrieval. rg --type go -A 10 'GetAllEventRecord' # Test: Search for the definition of Keeper. Expect: Find the struct or interface definition. rg --type go -A 10 'Keeper'Length of output: 304410
x/bnblightclient/types/params.go (2)
14-31: Ensure comprehensive validation inParams.Validate.The
Validatemethod checks theStakePlanHubAddress,EventName, andAllowList. Ensure that these validations are comprehensive and handle all edge cases.Verification successful
Comprehensive validation in
Params.Validateconfirmed.The
Params.Validatemethod includes appropriate checks forStakePlanHubAddress,EventName, andAllowList. The validations are comprehensive and handle the necessary edge cases.
common.IsHexAddressis used correctly to validate hex addresses.ValidateAllowListensures the allow list contains valid and unique addresses.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the comprehensiveness of validation in Params.Validate. # Test: Search for the usage of common.IsHexAddress. Expect: Correct validation of hex addresses. rg --type go -A 5 $'common.IsHexAddress' # Test: Search for the implementation of ValidateAllowList. Expect: Correct validation of allow list. rg --type go -A 10 $'func ValidateAllowList(allowList []string) error'Length of output: 7129
Script:
#!/bin/bash # Description: Verify the comprehensiveness of validation in Params.Validate. # Test: Search for the implementation of ValidateAllowList. Expect: Correct validation of allow list. rg --type go -A 10 'func ValidateAllowList\(allowList \[\]string\) error'Length of output: 788
37-50: Ensure correct validation inValidateAllowList.The
ValidateAllowListfunction checks for duplicate and invalid addresses. Ensure that the validation is correct and handles all edge cases.x/bnblightclient/keeper/prune.go (1)
8-17: LGTM!The function
pruneis well-implemented and follows the expected logic.x/bnblightclient/types/msg.go (3)
13-24: LGTM!The methods
ValidateBasicandGetSignersforMsgUpdateHeaderare well-implemented and follow the expected logic.
26-37: LGTM!The methods
ValidateBasicandGetSignersforMsgUploadHeadersare well-implemented and follow the expected logic.
39-49: LGTM!The methods
ValidateBasicandGetSignersforMsgUpdateParamsare well-implemented and follow the expected logic.proto/lorenzo/bnblightclient/v1/query.proto (9)
11-34: LGTM!The gRPC service
Queryis well-defined and follows the expected structure. The methods are appropriately annotated with HTTP options.
36-37: LGTM!The message type
QueryParamsRequestis well-defined and follows the expected structure.
39-43: LGTM!The message type
QueryParamsResponseis well-defined and follows the expected structure.
45-46: LGTM!The message type
QueryHeaderRequestis well-defined and follows the expected structure.
48-52: LGTM!The message type
QueryHeaderResponseis well-defined and follows the expected structure.
54-56: LGTM!The message type
QueryHeaderByHashRequestis well-defined and follows the expected structure.
58-63: LGTM!The message type
QueryHeaderByHashResponseis well-defined and follows the expected structure.
65-67: LGTM!The message type
QueryLatestHeaderRequestis well-defined and follows the expected structure.
69-73: LGTM!The message type
QueryLatestHeaderResponseis well-defined and follows the expected structure.x/bnblightclient/keeper/header.go (7)
45-55: LGTM!The
UpdateHeaderfunction is well-implemented with proper error handling and verification.
94-112: LGTM!The
GetHeaderfunction is well-implemented with proper error handling and unmarshalling.
114-132: LGTM!The
GetAllHeadersfunction is well-implemented with proper error handling and iteration.
135-153: LGTM!The
GetHeaderByHashfunction is well-implemented with proper error handling and unmarshalling.
155-164: LGTM!The
HasHeaderfunction is well-implemented and follows good practices.
166-173: LGTM!The
setHeaderfunction is well-implemented with proper marshalling and error handling.
175-179: LGTM!The
setLatestNumberfunction is well-implemented with proper marshalling and error handling.
| }) | ||
|
|
||
| vHeader := headers | ||
| latestedHeader, exist := k.GetLatestHeader(ctx) |
There was a problem hiding this comment.
Fix typo in variable name.
The variable name latestedHeader has a typo. It should be latestHeader.
- latestedHeader
+ latestHeaderCommittable 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.
| latestedHeader, exist := k.GetLatestHeader(ctx) | |
| latestHeader, exist := k.GetLatestHeader(ctx) |
x/bnblightclient/keeper/prune.go
Outdated
| func (k Keeper) pruneHeaders(ctx sdk.Context, pruneEndNumber uint64) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| iterator := sdk.KVStoreReversePrefixIterator(store, types.KeyPrefixHeader) | ||
| defer iterator.Close() | ||
|
|
||
| for ; iterator.Valid(); iterator.Next() { | ||
| iterKey := iterator.Key() | ||
| number := sdk.BigEndianToUint64(iterKey[1:]) | ||
| if number <= pruneEndNumber { | ||
| header, exist := k.GetHeader(ctx, number) | ||
| if !exist { | ||
| continue | ||
| } | ||
|
|
||
| // delete header | ||
| store.Delete(iterKey) | ||
| // delete header hash | ||
| store.Delete(types.KeyHeaderHash(header.Hash)) | ||
|
|
||
| // delete event record | ||
| prefix := append(types.KeyPrefixEventRecord, sdk.Uint64ToBigEndian(number)...) | ||
| iterator2 := sdk.KVStoreReversePrefixIterator(store, prefix) | ||
| defer iterator2.Close() | ||
|
|
||
| for ; iterator2.Valid(); iterator2.Next() { | ||
| iterKey2 := iterator2.Key() | ||
| store.Delete(iterKey2) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Check error return values of iterator.Close and iterator2.Close.
The error return values of iterator.Close and iterator2.Close are not checked. This can lead to resource leaks or unhandled errors.
- defer iterator.Close()
+ defer func() {
+ if err := iterator.Close(); err != nil {
+ // handle error
+ }
+ }()
- defer iterator2.Close()
+ defer func() {
+ if err := iterator2.Close(); err != nil {
+ // handle error
+ }
+ }()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.
| func (k Keeper) pruneHeaders(ctx sdk.Context, pruneEndNumber uint64) { | |
| store := ctx.KVStore(k.storeKey) | |
| iterator := sdk.KVStoreReversePrefixIterator(store, types.KeyPrefixHeader) | |
| defer iterator.Close() | |
| for ; iterator.Valid(); iterator.Next() { | |
| iterKey := iterator.Key() | |
| number := sdk.BigEndianToUint64(iterKey[1:]) | |
| if number <= pruneEndNumber { | |
| header, exist := k.GetHeader(ctx, number) | |
| if !exist { | |
| continue | |
| } | |
| // delete header | |
| store.Delete(iterKey) | |
| // delete header hash | |
| store.Delete(types.KeyHeaderHash(header.Hash)) | |
| // delete event record | |
| prefix := append(types.KeyPrefixEventRecord, sdk.Uint64ToBigEndian(number)...) | |
| iterator2 := sdk.KVStoreReversePrefixIterator(store, prefix) | |
| defer iterator2.Close() | |
| for ; iterator2.Valid(); iterator2.Next() { | |
| iterKey2 := iterator2.Key() | |
| store.Delete(iterKey2) | |
| } | |
| } | |
| } | |
| } | |
| func (k Keeper) pruneHeaders(ctx sdk.Context, pruneEndNumber uint64) { | |
| store := ctx.KVStore(k.storeKey) | |
| iterator := sdk.KVStoreReversePrefixIterator(store, types.KeyPrefixHeader) | |
| defer func() { | |
| if err := iterator.Close(); err != nil { | |
| // handle error | |
| } | |
| }() | |
| for ; iterator.Valid(); iterator.Next() { | |
| iterKey := iterator.Key() | |
| number := sdk.BigEndianToUint64(iterKey[1:]) | |
| if number <= pruneEndNumber { | |
| header, exist := k.GetHeader(ctx, number) | |
| if !exist { | |
| continue | |
| } | |
| // delete header | |
| store.Delete(iterKey) | |
| // delete header hash | |
| store.Delete(types.KeyHeaderHash(header.Hash)) | |
| // delete event record | |
| prefix := append(types.KeyPrefixEventRecord, sdk.Uint64ToBigEndian(number)...) | |
| iterator2 := sdk.KVStoreReversePrefixIterator(store, prefix) | |
| defer func() { | |
| if err := iterator2.Close(); err != nil { | |
| // handle error | |
| } | |
| }() | |
| for ; iterator2.Valid(); iterator2.Next() { | |
| iterKey2 := iterator2.Key() | |
| store.Delete(iterKey2) | |
| } | |
| } | |
| } | |
| } |
Tools
golangci-lint
22-22: Error return value of
iterator.Closeis not checked(errcheck)
41-41: Error return value of
iterator2.Closeis not checked(errcheck)
GitHub Check: golangci-lint
[failure] 22-22:
Error return value ofiterator.Closeis not checked (errcheck)
[failure] 41-41:
Error return value ofiterator2.Closeis not checked (errcheck)
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
proto/lorenzo/bnblightclient/v1/genesis.proto (2)
9-17: Ensure comprehensive documentation for message fields.The
GenesisStatemessage defines parameters, headers, and events. Ensure that each field is well-documented to provide clarity on its purpose and usage.// GenesisState defines the bnb light client state message GenesisState { // params defines the bnb light client parameters lorenzo.bnblightclient.v1.Params params = 1; // headers defines the bnb light client headers repeated lorenzo.bnblightclient.v1.Header headers = 2; // events defines the bnb chain generated events repeated EvmEvent events = 3; }
19-25: Ensure comprehensive documentation for message fields.The
EvmEventmessage defines the block number, contract, and identifier. Ensure that each field is well-documented to provide clarity on its purpose and usage.message EvmEvent { // block_number defines the block number uint64 block_number = 1; // contract defines the contract bytes contract = 2; // identifier defines the unique identifier of the event uint64 identifier = 3; }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/bnblightclient/types/genesis.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- proto/lorenzo/bnblightclient/v1/genesis.proto (1 hunks)
- x/bnblightclient/client/cli/query.go (1 hunks)
- x/bnblightclient/client/cli/tx.go (1 hunks)
- x/bnblightclient/keeper/genesis.go (1 hunks)
- x/bnblightclient/keeper/grpc_query.go (1 hunks)
- x/bnblightclient/keeper/keeper.go (1 hunks)
- x/bnblightclient/keeper/proof.go (1 hunks)
- x/bnblightclient/module.go (1 hunks)
- x/bnblightclient/types/codec.go (1 hunks)
- x/bnblightclient/types/events.go (1 hunks)
- x/bnblightclient/types/genesis.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- x/bnblightclient/keeper/genesis.go
- x/bnblightclient/keeper/grpc_query.go
- x/bnblightclient/keeper/keeper.go
- x/bnblightclient/keeper/proof.go
Additional context used
golangci-lint
x/bnblightclient/types/genesis.go
16-16: File is not
gofumpt-ed(gofumpt)
18-18: File is not
gofumpt-ed(gofumpt)
x/bnblightclient/types/events.go
24-24: File is not
gofumpt-ed(gofumpt)
Additional comments not posted (22)
x/bnblightclient/types/genesis.go (2)
6-11: Ensure comprehensive validation of headers.The
Validatefunction checks the headers usingVeryHeaders, which is not defined in this file. Ensure thatVeryHeadersperforms thorough validation of the headers to prevent potential issues.
13-19: Ensure default values are appropriate.The
DefaultGenesisStatefunction initializes the genesis state with empty parameters, headers, and events. Verify that these defaults are appropriate for the application's requirements.Verification successful
Ensure default values are appropriate.
The
DefaultGenesisStatefunction initializes the genesis state with empty parameters, headers, and events. The usage ofDefaultGenesisStatein the codebase, as identified, appears to be primarily for JSON marshaling and validation purposes. These usages suggest that the default values are appropriate and handled correctly within the application.
- Files and lines to review:
x/bnblightclient/module.go: Line withreturn cdc.MustMarshalJSON(types.DefaultGenesisState())x/plan/module.go: Line withreturn cdc.MustMarshalJSON(types.DefaultGenesisState())x/plan/keeper/genesis_test.go: Lines withtypes.DefaultGenesisState()x/plan/types/genesis.go: Line withfunc DefaultGenesisState() *GenesisStatex/bnblightclient/types/genesis.go: Line withfunc DefaultGenesisState() *GenesisStatex/agent/types/genesis.go: Line withfunc DefaultGenesisState() *GenesisStatex/fee/module.go: Line withreturn cdc.MustMarshalJSON(types.DefaultGenesisState())x/fee/types/msgs.go: Line withfunc DefaultGenesisState() *GenesisStatex/agent/module.go: Line withreturn cdc.MustMarshalJSON(types.DefaultGenesisState())app/genesis.go: Line withfunc NewDefaultGenesisState(cdc codec.JSONCodec) GenesisStateapp/helpers/test_helper.go: Lines withreturn LorenzoApp, app.NewDefaultGenesisState(encConfig.Codec)andbanktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, totalSupply, []banktypes.Metadata{}, []banktypes.SendEnabled{})The default values set in
DefaultGenesisStateappear to be appropriate and are used consistently across the codebase for initializing and validating the genesis state.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and appropriateness of default values in `DefaultGenesisState`. # Test: Search for the usage of `DefaultGenesisState`. Expect: Appropriate handling of default values. rg --type go -A 5 $'DefaultGenesisState'Length of output: 5151
Tools
golangci-lint
16-16: File is not
gofumpt-ed(gofumpt)
18-18: File is not
gofumpt-ed(gofumpt)
x/bnblightclient/types/codec.go (1)
13-19: Ensure all necessary message types are registered.The
RegisterInterfacesfunction registersMsgUpdateParams,MsgUpdateHeader, andMsgUploadHeaders. Verify that all necessary message types are included and that the service descriptor registration is correct.Verification successful
Ensure all necessary message types are registered.
The
RegisterInterfacesfunction registersMsgUpdateParams,MsgUpdateHeader, andMsgUploadHeaders. Verify that all necessary message types are included and that the service descriptor registration is correct.
Ensure all necessary message types are registered.
The
RegisterInterfacesfunction registersMsgUpdateParams,MsgUpdateHeader, andMsgUploadHeaders. The response typesMsgUploadHeadersResponse,MsgUpdateHeaderResponse, andMsgUpdateParamsResponseare not registered, which might be intentional. Verify if it is standard practice to exclude response types from registration.
Ensure all necessary message types are registered.
The
RegisterInterfacesfunction correctly registers the necessary message types:MsgUpdateParams,MsgUpdateHeader, andMsgUploadHeaders. It follows the standard practice of excluding response types from registration.
- No changes needed.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the completeness of message type registrations in `RegisterInterfaces`. # Test: Search for the usage of `RegisterInterfaces`. Expect: All necessary message types are registered. rg --type go -A 5 $'RegisterInterfaces'Length of output: 8516
Script:
#!/bin/bash # Description: Verify the completeness of message type registrations in `RegisterInterfaces` for bnblightclient. # Search for all message types defined in the bnblightclient module. rg --type go 'type Msg' -A 2Length of output: 13754
Script:
#!/bin/bash # Description: Verify if other modules exclude response types from registration. # Search for response types in the `plan` module and check their registration. rg --type go 'type Msg' -A 2 x/plan/types/tx.pb.goLength of output: 2629
x/bnblightclient/types/events.go (2)
1-8: LGTM!The package declaration and imports look good.
10-11: Reminder: Complete the ABI definition.The TODO comment indicates that the ABI for the StakePlanHub contract is not yet defined.
x/bnblightclient/client/cli/tx.go (4)
1-14: LGTM!The package declaration and imports look good.
16-28: LGTM!The GetTxCmd function is well-structured and correctly adds subcommands.
30-74: LGTM!The CmdTxUploadHeaders function is well-documented and handles errors appropriately.
76-120: LGTM!The CmdTxUpdateHeader function is well-documented and handles errors appropriately.
x/bnblightclient/client/cli/query.go (6)
1-14: LGTM!The package declaration and imports look good.
16-30: LGTM!The GetQueryCmd function is well-structured and correctly adds subcommands.
32-53: LGTM!The CmdQueryParams function is well-documented and handles errors appropriately.
55-81: LGTM!The CmdQueryHeader function is well-documented and handles errors appropriately.
83-108: LGTM!The CmdQueryHeaderHash function is well-documented and handles errors appropriately.
110-135: LGTM!The CmdQueryLatestHeader function is well-documented and handles errors appropriately.
x/bnblightclient/module.go (7)
3-21: Imports look good.The imported packages are appropriate and necessary for the functionality provided in the file.
32-88: AppModuleBasic struct and methods look good.The methods in
AppModuleBasicare well-defined and follow the standard patterns for module implementation in Cosmos SDK.
94-116: AppModule struct and methods look good.The methods in
AppModuleare well-defined and follow the standard patterns for module implementation in Cosmos SDK.
148-155: Consider implementing BeginBlock and EndBlock methods.The
BeginBlockandEndBlockmethods are currently empty. These methods can be used to execute logic at the beginning and end of each block, respectively.Are there any plans to implement logic in these methods in the future?
37-46: NewAppModuleBasic function looks good.The constructor is well-defined and follows the standard patterns for module implementation in Cosmos SDK.
101-117: NewAppModule function looks good.The constructor is well-defined and follows the standard patterns for module implementation in Cosmos SDK.
119-159: AppModule methods look good.The methods are well-defined and follow the standard patterns for module implementation in Cosmos SDK.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- x/bnblightclient/keeper/proof.go (1 hunks)
- x/bnblightclient/types/errors.go (1 hunks)
- x/bnblightclient/types/events.go (1 hunks)
- x/bnblightclient/types/stake_plan_hub_abi.json (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/bnblightclient/keeper/proof.go
- x/bnblightclient/types/errors.go
Additional context used
golangci-lint
x/bnblightclient/types/events.go
6-6: File is not
gofumpt-ed(gofumpt)
14-14: File is not
gofumpt-ed(gofumpt)
42-42: File is not
gofumpt-ed(gofumpt)
Additional comments not posted (7)
x/bnblightclient/types/events.go (4)
3-11: LGTM!The import statements are appropriate and necessary for the functionality provided in the file.
Tools
golangci-lint
6-6: File is not
gofumpt-ed(gofumpt)
13-16: LGTM!The variable declarations for the ABI JSON and the ABI object are appropriate and necessary.
Tools
golangci-lint
14-14: File is not
gofumpt-ed(gofumpt)
26-29: LGTM!The
ABIstakePlanHubfunction is appropriate for providing access to the ABI object.
31-39: LGTM!The
CrossChainEventstruct definition is appropriate for representing a cross-chain event.x/bnblightclient/types/stake_plan_hub_abi.json (3)
1-78: LGTM!The constructor and error definitions are appropriate for the contract.
101-419: LGTM!The event definitions are appropriate for the contract.
421-797: LGTM!The function definitions are appropriate for the contract.
Summary by CodeRabbit
New Features
Bug Fixes
Chores