-
Notifications
You must be signed in to change notification settings - Fork 4
feat: fix IBC #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: fix IBC #101
Changes from all commits
4a00e43
90b41ee
c02a0bb
52d17df
0496245
3536797
59a24ad
be850b6
b1d2d4c
a842ae1
c0979a0
9f3846e
9a250bb
b746bce
4585ebc
38a4ab1
d01f033
0c0ce26
450fec3
4d84d69
9d99a5d
24b1479
ca03b26
f41ee25
9990da1
0986c52
1279f30
92215fa
61970f2
ec24c05
9441582
c926081
9f8fbe8
b787678
f5b0dc6
f8a8fc2
0fdcd7d
405feb9
42e7753
58d0fe3
f5b10ff
b0d88ce
c6a6b1b
6d7d2d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,10 @@ import ( | |
| "cosmossdk.io/log" | ||
| abci "github.com/cometbft/cometbft/abci/types" | ||
| "github.com/cometbft/cometbft/config" | ||
| "github.com/cometbft/cometbft/libs/bytes" | ||
| "github.com/cometbft/cometbft/mempool" | ||
| corep2p "github.com/cometbft/cometbft/p2p" | ||
| cmtprototypes "github.com/cometbft/cometbft/proto/tendermint/types" | ||
| cmtstate "github.com/cometbft/cometbft/state" | ||
| cmttypes "github.com/cometbft/cometbft/types" | ||
| servertypes "github.com/cosmos/cosmos-sdk/server/types" | ||
|
|
@@ -25,7 +27,9 @@ import ( | |
| rollnode "github.com/rollkit/rollkit/node" | ||
| rollkitp2p "github.com/rollkit/rollkit/pkg/p2p" | ||
| rstore "github.com/rollkit/rollkit/pkg/store" | ||
| "github.com/rollkit/rollkit/types" | ||
|
|
||
| "github.com/rollkit/go-execution-abci/pkg/cometcompat" | ||
| "github.com/rollkit/go-execution-abci/pkg/p2p" | ||
| ) | ||
|
|
||
|
|
@@ -244,6 +248,7 @@ func (a *Adapter) InitChain(ctx context.Context, genesisTime time.Time, initialH | |
| } else { | ||
| s.ConsensusParams = cmttypes.ConsensusParamsFromProto(consensusParams) | ||
| } | ||
| s.ChainID = chainID | ||
|
|
||
| vals, err := cmttypes.PB2TM.ValidatorUpdates(res.Validators) | ||
| if err != nil { | ||
|
|
@@ -292,14 +297,61 @@ func (a *Adapter) ExecuteTxs( | |
| return nil, 0, fmt.Errorf("failed to load state: %w", err) | ||
| } | ||
|
|
||
| header, ok := types.SignedHeaderFromContext(ctx) | ||
| if !ok { | ||
| return nil, 0, fmt.Errorf("rollkit header not found in context") | ||
| } | ||
|
|
||
| var proposedLastCommit abci.CommitInfo | ||
| var lastCommit *cmttypes.Commit | ||
|
|
||
| if blockHeight > 1 { | ||
| header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1) | ||
| if err != nil { | ||
| return nil, 0, fmt.Errorf("failed to get previous block data: %w", err) | ||
| } | ||
|
|
||
| commitForPrevBlock := &cmttypes.Commit{ | ||
| Height: int64(header.Height()), | ||
| Round: 0, | ||
| BlockID: cmttypes.BlockID{Hash: bytes.HexBytes(header.Hash()), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: bytes.HexBytes(data.Hash())}}, | ||
| Signatures: []cmttypes.CommitSig{ | ||
| { | ||
| BlockIDFlag: cmttypes.BlockIDFlagCommit, | ||
| ValidatorAddress: cmttypes.Address(header.ProposerAddress), | ||
| Timestamp: header.Time(), | ||
| Signature: header.Signature, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| lastCommit = commitForPrevBlock | ||
| proposedLastCommit = cometCommitToABCICommitInfo(commitForPrevBlock) | ||
| } else { | ||
| // For the first block, ProposedLastCommit is empty | ||
| proposedLastCommit = abci.CommitInfo{Round: 0, Votes: []abci.VoteInfo{}} | ||
| lastCommit = &cmttypes.Commit{ | ||
| Height: int64(blockHeight), | ||
| Round: 0, | ||
| BlockID: cmttypes.BlockID{}, | ||
| Signatures: []cmttypes.CommitSig{}, | ||
| } | ||
| } | ||
|
|
||
| emptyBlock, err := cometcompat.ToABCIBlock(header, &types.Data{}, lastCommit) | ||
| if err != nil { | ||
| return nil, 0, fmt.Errorf("failed to compute header hash: %w", err) | ||
| } | ||
|
|
||
| ppResp, err := a.App.ProcessProposal(&abci.RequestProcessProposal{ | ||
| Txs: txs, | ||
| ProposedLastCommit: abci.CommitInfo{}, | ||
| Hash: prevStateRoot, | ||
| Hash: emptyBlock.Header.Hash(), | ||
| Height: int64(blockHeight), | ||
| Time: timestamp, | ||
| NextValidatorsHash: s.NextValidators.Hash(), | ||
| Txs: txs, | ||
| ProposedLastCommit: proposedLastCommit, | ||
| Misbehavior: []abci.Misbehavior{}, | ||
| ProposerAddress: s.Validators.Proposer.Address, | ||
| NextValidatorsHash: s.NextValidators.Hash(), | ||
| }) | ||
|
Comment on lines
353
to
355
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Use the proposer address from the Rollkit header: - ProposerAddress: s.Validators.Proposer.Address,
+ ProposerAddress: header.ProposerAddress,Also applies to: 357-359 🤖 Prompt for AI Agents |
||
| if err != nil { | ||
| return nil, 0, err | ||
|
|
@@ -310,17 +362,24 @@ func (a *Adapter) ExecuteTxs( | |
| } | ||
|
|
||
| fbResp, err := a.App.FinalizeBlock(&abci.RequestFinalizeBlock{ | ||
| Txs: txs, | ||
| Hash: prevStateRoot, | ||
| Height: int64(blockHeight), | ||
| Time: timestamp, | ||
| Hash: emptyBlock.Header.Hash(), | ||
| NextValidatorsHash: s.NextValidators.Hash(), | ||
| ProposerAddress: s.Validators.Proposer.Address, | ||
| Height: int64(blockHeight), | ||
| Time: timestamp, | ||
| DecidedLastCommit: abci.CommitInfo{ | ||
| Round: 0, | ||
| Votes: nil, | ||
| }, | ||
| Txs: txs, | ||
| }) | ||
| if err != nil { | ||
| return nil, 0, err | ||
| } | ||
|
|
||
| s.AppHash = fbResp.AppHash | ||
| s.LastBlockHeight = int64(blockHeight) | ||
|
|
||
| nValSet := s.NextValidators.Copy() | ||
|
|
||
| validatorUpdates, err := cmttypes.PB2TM.ValidatorUpdates(fbResp.ValidatorUpdates) | ||
|
|
@@ -404,8 +463,46 @@ func (a *Adapter) ExecuteTxs( | |
| for i := range txs { | ||
| cmtTxs[i] = txs[i] | ||
| } | ||
| block := s.MakeBlock(int64(blockHeight), cmtTxs, &cmttypes.Commit{Height: int64(blockHeight)}, nil, s.Validators.Proposer.Address) | ||
| fireEvents(a.Logger, a.EventBus, block, cmttypes.BlockID{}, fbResp, validatorUpdates) | ||
|
|
||
| commit := &cmttypes.Commit{ | ||
| Height: int64(blockHeight), | ||
| Round: 0, | ||
| Signatures: []cmttypes.CommitSig{ | ||
| { | ||
| BlockIDFlag: cmttypes.BlockIDFlagCommit, | ||
| ValidatorAddress: s.Validators.Proposer.Address, | ||
| Timestamp: time.Now().UTC(), | ||
| Signature: []byte{}, | ||
| }, | ||
| }, | ||
| } | ||
|
Comment on lines
+467
to
+478
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Genesis-path commit invalid: mismatched timestamp & empty signature For
Both produce a non-canonical commit hash and may break light-client verification. 🤖 Prompt for AI Agents |
||
|
|
||
| if blockHeight > 1 { | ||
| header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1) | ||
| if err != nil { | ||
| return nil, 0, fmt.Errorf("failed to get previous block data: %w", err) | ||
| } | ||
|
|
||
| commit = &cmttypes.Commit{ | ||
| Height: int64(header.Height()), | ||
| Round: 0, | ||
| BlockID: cmttypes.BlockID{Hash: bytes.HexBytes(header.Hash()), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: bytes.HexBytes(data.Hash())}}, | ||
| Signatures: []cmttypes.CommitSig{ | ||
| { | ||
| BlockIDFlag: cmttypes.BlockIDFlagCommit, | ||
| ValidatorAddress: cmttypes.Address(header.ProposerAddress), | ||
| Timestamp: header.Time(), | ||
| Signature: header.Signature, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| block := s.MakeBlock(int64(blockHeight), cmtTxs, commit, nil, s.Validators.Proposer.Address) | ||
|
|
||
| currentBlockID := cmttypes.BlockID{Hash: block.Hash(), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: block.DataHash}} | ||
|
|
||
| fireEvents(a.Logger, a.EventBus, block, currentBlockID, fbResp, validatorUpdates) | ||
|
|
||
| a.Logger.Info("block executed successfully", "height", blockHeight, "appHash", fmt.Sprintf("%X", fbResp.AppHash)) | ||
| return fbResp.AppHash, uint64(s.ConsensusParams.Block.MaxBytes), nil | ||
|
|
@@ -521,3 +618,34 @@ func (a *Adapter) GetTxs(ctx context.Context) ([][]byte, error) { | |
| func (a *Adapter) SetFinal(ctx context.Context, blockHeight uint64) error { | ||
| return nil | ||
| } | ||
|
|
||
| func cometCommitToABCICommitInfo(commit *cmttypes.Commit) abci.CommitInfo { | ||
| if commit == nil { | ||
| return abci.CommitInfo{ | ||
| Round: 0, | ||
| Votes: []abci.VoteInfo{}, | ||
| } | ||
| } | ||
|
|
||
| if len(commit.Signatures) == 0 { | ||
| return abci.CommitInfo{ | ||
| Round: commit.Round, | ||
| Votes: []abci.VoteInfo{}, | ||
| } | ||
| } | ||
|
|
||
| votes := make([]abci.VoteInfo, len(commit.Signatures)) | ||
| for i, sig := range commit.Signatures { | ||
| votes[i] = abci.VoteInfo{ | ||
| Validator: abci.Validator{ | ||
| Address: sig.ValidatorAddress, | ||
| Power: 0, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @randygrok |
||
| }, | ||
| BlockIdFlag: cmtprototypes.BlockIDFlag(sig.BlockIDFlag), | ||
| } | ||
| } | ||
|
Comment on lines
+637
to
+646
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVoteInfo carries zero power – may break consensus/hash
Populate 🏁 Script executed: #!/bin/bash
# Display the cometCommitToABCICommitInfo function and its context
rg -n "func cometCommitToABCICommitInfo" -A50 -B10 pkg/adapter/adapter.goLength of output: 1233 🏁 Script executed: #!/bin/bash
# Search for the definition of Adapter in pkg/adapter/adapter.go
rg -n "type .*Adapter" -A5 -B5 pkg/adapter/adapter.goLength of output: 376 🏁 Script executed: #!/bin/bash
# Display the BeginBlock method implementation in adapter.go
rg -n "func (a .*Adapter) BeginBlock" -A50 -B10 pkg/adapter/adapter.goLength of output: 72 🏁 Script executed: #!/bin/bash
# Display the first 200 lines of pkg/adapter/adapter.go to inspect imports and types
sed -n '1,200p' pkg/adapter/adapter.goLength of output: 6190 🏁 Script executed: #!/bin/bash
# Find Info method implementation
rg -n 'func \(a \*Adapter\) Info' -A50 -B10 pkg/adapter/adapter.go
# Check if cometCommitToABCICommitInfo is referenced elsewhere
rg -n 'cometCommitToABCICommitInfo' -n pkg/adapter/adapter.goLength of output: 285 Populate The helper • Load the validator set for the given height (e.g. via votes[i] = abci.VoteInfo{
Validator: abci.Validator{
Address: sig.ValidatorAddress,
- Power: 0,
+ Power: validator.VotingPower, // load from state
},
BlockIdFlag: cmtprototypes.BlockIDFlag(sig.BlockIDFlag),
}• If you cannot fetch validator power here, omit the This change is required to ensure correct consensus and hash calculations.
🤖 Prompt for AI Agents |
||
| return abci.CommitInfo{ | ||
| Round: commit.Round, | ||
| Votes: votes, | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Commit construction duplicated & partially inconsistent
The logic that fabricates a commit for the previous block appears twice (lines 317-329 and 476-488).
Apart from code repetition, the two snippets risk drifting apart (e.g. timestamp handling). Extract a helper (e.g.
buildCommitFromPreviousBlock) and invoke it in both places.Also applies to: 476-488
🤖 Prompt for AI Agents