-
Notifications
You must be signed in to change notification settings - Fork 3
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
base: main
Are you sure you want to change the base?
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
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" | ||||||
types1 "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,6 +27,7 @@ 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/p2p" | ||||||
) | ||||||
|
@@ -244,6 +247,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 { | ||||||
|
@@ -279,6 +283,7 @@ func (a *Adapter) ExecuteTxs( | |||||
blockHeight uint64, | ||||||
timestamp time.Time, | ||||||
prevStateRoot []byte, | ||||||
metadata map[string]interface{}, | ||||||
) ([]byte, uint64, error) { | ||||||
execStart := time.Now() | ||||||
defer func() { | ||||||
|
@@ -287,19 +292,53 @@ func (a *Adapter) ExecuteTxs( | |||||
a.Logger.Info("Executing block", "height", blockHeight, "num_txs", len(txs), "timestamp", timestamp) | ||||||
a.Metrics.TxsExecutedPerBlock.Observe(float64(len(txs))) | ||||||
|
||||||
var headerHash types.Hash | ||||||
if h, ok := metadata[types.HeaderHashKey]; ok { | ||||||
headerHash = h.(types.Hash) | ||||||
} else { | ||||||
a.Logger.Info("header hash not found in metadata, running genesis block") | ||||||
} | ||||||
|
||||||
s, err := a.Store.LoadState(ctx) | ||||||
if err != nil { | ||||||
return nil, 0, fmt.Errorf("failed to load state: %w", err) | ||||||
} | ||||||
|
||||||
var proposedLastCommit abci.CommitInfo | ||||||
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, | ||||||
}, | ||||||
}, | ||||||
} | ||||||
proposedLastCommit = cometCommitToABCICommitInfo(commitForPrevBlock) | ||||||
} else { | ||||||
// For the first block, ProposedLastCommit is empty | ||||||
proposedLastCommit = abci.CommitInfo{Round: 0, Votes: []abci.VoteInfo{}} | ||||||
} | ||||||
|
||||||
ppResp, err := a.App.ProcessProposal(&abci.RequestProcessProposal{ | ||||||
Txs: txs, | ||||||
ProposedLastCommit: abci.CommitInfo{}, | ||||||
Hash: prevStateRoot, | ||||||
Hash: headerHash, | ||||||
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(), | ||||||
}) | ||||||
if err != nil { | ||||||
return nil, 0, err | ||||||
|
@@ -310,17 +349,24 @@ func (a *Adapter) ExecuteTxs( | |||||
} | ||||||
|
||||||
fbResp, err := a.App.FinalizeBlock(&abci.RequestFinalizeBlock{ | ||||||
Txs: txs, | ||||||
Hash: prevStateRoot, | ||||||
Height: int64(blockHeight), | ||||||
Time: timestamp, | ||||||
Hash: headerHash, | ||||||
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 +450,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) | ||||||
|
||||||
var commit = &cmttypes.Commit{ | ||||||
Height: int64(blockHeight), | ||||||
Round: 0, | ||||||
Signatures: []cmttypes.CommitSig{ | ||||||
{ | ||||||
BlockIDFlag: cmttypes.BlockIDFlagCommit, | ||||||
ValidatorAddress: s.Validators.Proposer.Address, | ||||||
Timestamp: time.Now(), | ||||||
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. The timestamp for the commit signature should match the block's timestamp rather than using
Suggested change
Spotted by Diamond |
||||||
Signature: []byte{}, | ||||||
}, | ||||||
}, | ||||||
} | ||||||
|
||||||
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, | ||||||
}, | ||||||
}, | ||||||
} | ||||||
} | ||||||
Comment on lines
+454
to
+486
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. 🛠️ Refactor suggestion Redundant commit construction logic. The commit construction logic appears twice - once for Consider extracting the commit construction into a helper function: +func (a *Adapter) buildCommitFromPreviousBlock(ctx context.Context, blockHeight uint64) (*cmttypes.Commit, error) {
+ if blockHeight <= 1 {
+ return &cmttypes.Commit{
+ Height: int64(blockHeight),
+ Round: 0,
+ Signatures: []cmttypes.CommitSig{
+ {
+ BlockIDFlag: cmttypes.BlockIDFlagCommit,
+ ValidatorAddress: nil, // or appropriate default
+ Timestamp: time.Now(),
+ Signature: []byte{},
+ },
+ },
+ }, nil
+ }
+
+ header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1)
+ if err != nil {
+ return nil, fmt.Errorf("failed to get previous block data: %w", err)
+ }
+
+ return &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,
+ },
+ },
+ }, nil
+} Then use it in both places to eliminate duplication.
🤖 Prompt for AI Agents
|
||||||
|
||||||
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 +605,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, | ||||||
}, | ||||||
BlockIdFlag: types1.BlockIDFlag(sig.BlockIDFlag), | ||||||
} | ||||||
} | ||||||
return abci.CommitInfo{ | ||||||
Round: commit.Round, | ||||||
Votes: votes, | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,16 @@ | ||||||||||||||||||||||||||||||
package rollkitadapter | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||
"github.com/rollkit/rollkit/types" | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
"github.com/rollkit/go-execution-abci/pkg/rpc" | ||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
func CreateCometBFTCommitHasher() types.CommitHashProvider { | ||||||||||||||||||||||||||||||
return func(signature *types.Signature, header *types.Header, proposerAddress []byte) (types.Hash, error) { | ||||||||||||||||||||||||||||||
abciCommit := rpc.GetABCICommit(header.Height(), header.Hash(), proposerAddress, header.Time(), *signature) | ||||||||||||||||||||||||||||||
abciCommit.Signatures[0].ValidatorAddress = proposerAddress | ||||||||||||||||||||||||||||||
abciCommit.Signatures[0].Timestamp = header.Time() | ||||||||||||||||||||||||||||||
return types.Hash(abciCommit.Hash()), nil | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+9
to
+16
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. 🛠️ Refactor suggestion Remove redundant field assignments. Lines 12-13 are redundant as
Apply this diff to remove the redundant assignments: func CreateCometBFTCommitHasher() types.CommitHashProvider {
return func(signature *types.Signature, header *types.Header, proposerAddress []byte) (types.Hash, error) {
abciCommit := rpc.GetABCICommit(header.Height(), header.Hash(), proposerAddress, header.Time(), *signature)
- abciCommit.Signatures[0].ValidatorAddress = proposerAddress
- abciCommit.Signatures[0].Timestamp = header.Time()
return types.Hash(abciCommit.Hash()), nil
}
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package rollkitadapter | ||
|
||
import ( | ||
"github.com/rollkit/rollkit/types" | ||
|
||
"github.com/rollkit/go-execution-abci/pkg/common" | ||
) | ||
|
||
func CreateCometBFTHeaderHasher() types.HeaderHasher { | ||
return func(header *types.Header) (types.Hash, error) { | ||
abciHeader, err := common.ToABCIHeader(header) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return types.Hash(abciHeader.Hash()), nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package rollkitadapter | ||
|
||
import ( | ||
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" | ||
cmtypes "github.com/cometbft/cometbft/types" | ||
|
||
"github.com/rollkit/rollkit/types" | ||
|
||
"github.com/rollkit/go-execution-abci/pkg/rpc" | ||
) | ||
|
||
func CreateCometBFTPayloadProvider() types.SignaturePayloadProvider { | ||
return func(header *types.Header, data *types.Data) ([]byte, error) { | ||
abciHeaderForSigning, err := rpc.ToABCIHeader(header) | ||
if err != nil { | ||
return nil, err | ||
} | ||
vote := cmtproto.Vote{ | ||
Type: cmtproto.PrecommitType, | ||
Height: int64(header.Height()), //nolint:gosec | ||
Round: 0, | ||
BlockID: cmtproto.BlockID{ | ||
Hash: abciHeaderForSigning.Hash(), | ||
PartSetHeader: cmtproto.PartSetHeader{}, | ||
}, | ||
Timestamp: header.Time(), | ||
ValidatorAddress: header.ProposerAddress, | ||
ValidatorIndex: 0, | ||
} | ||
chainID := header.ChainID() | ||
consensusVoteBytes := cmtypes.VoteSignBytes(chainID, &vote) | ||
|
||
return consensusVoteBytes, nil | ||
} | ||
} |
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
Consider adding validation for the header hash type assertion.
While the code handles the case when header hash is not found (genesis block), the type assertion on line 297 could panic if the metadata contains an unexpected type.
Add type assertion safety check:
📝 Committable suggestion
🤖 Prompt for AI Agents