Pr comments

x/btccheckpoint/btcutils/btcutils.go

@@ -43,13 +43,13 @@ func readBlockHeader(headerBytes []byte) (*wire.BlockHeader, error) {
  return header, nil
 }
 
-// TODO copy of the validation done in btc light client at some point it would
-// be nice to move it to some commong btc module
+// Perform the checks that [checkBlockHeaderSanity](https://github.com/btcsuite/btcd/blob/master/blockchain/validate.go#L430) of btcd does
+//
+// We skip the "timestamp should not be 2 hours into the future" check
+// since this might introduce undeterministic behavior
 func validateHeader(header *wire.BlockHeader, powLimit *big.Int) error {
- // Perform the checks that checkBlockHeaderSanity of btcd does
- // https://github.com/btcsuite/btcd/blob/master/blockchain/validate.go#L430
- // We skip the "timestamp should not be 2 hours into the future" check
- // since this might introduce undeterministic behavior
+ // TODO copy of the validation done in btc light client at some point it would
+ // be nice to move it to some commong btc module
 
  msgBlock := &wire.MsgBlock{Header: *header}
 
@@ -80,12 +80,25 @@ func hashConcat(a []byte, b []byte) chainhash.Hash {
  return chainhash.HashH(c)
 }
 
+// Prove checks the validity of a merkle proof
 // proof logic copied from:
 // https://github.com/summa-tx/bitcoin-spv/blob/fb2a61e7a941d421ae833789d97ed10d2ad79cfe/golang/btcspv/bitcoin_spv.go#L498
 // main reason for not bringing library in, is that we already use btcd
 // bitcoin primitives and this library defines their own which could lead
 // to some mixups
-func verifyProof(proof []byte, index uint32) bool {
+func prove(tx *btcutil.Tx, merkleRoot *chainhash.Hash, intermediateNodes []byte, index uint32) bool {
+ txHash := tx.Hash()
+
+ // Shortcut the empty-block case
+ if txHash.IsEqual(merkleRoot) && index == 0 && len(intermediateNodes) == 0 {
+ return true
+ }
+
+ proof := []byte{}
+ proof = append(proof, txHash[:]...)
+ proof = append(proof, intermediateNodes...)
+ proof = append(proof, merkleRoot[:]...)
+
  var current chainhash.Hash
  idx := index
  proofLength := len(proof)
@@ -94,10 +107,6 @@ func verifyProof(proof []byte, index uint32) bool {
  return false
  }
 
- if proofLength == 32 {
- return true
- }
-
  if proofLength == 64 {
  return false
  }
@@ -124,23 +133,6 @@ func verifyProof(proof []byte, index uint32) bool {
  return bytes.Equal(current[:], root)
 }
 
-// Prove checks the validity of a merkle proof
-func prove(tx *btcutil.Tx, merkleRoot *chainhash.Hash, intermediateNodes []byte, index uint32) bool {
- txHash := tx.Hash()
-
- // Shortcut the empty-block case
- if txHash.IsEqual(merkleRoot) && index == 0 && len(intermediateNodes) == 0 {
- return true
- }
-
- proof := []byte{}
- proof = append(proof, txHash[:]...)
- proof = append(proof, intermediateNodes...)
- proof = append(proof, merkleRoot[:]...)
-
- return verifyProof(proof, index)
-}
-
 func extractOpReturnData(tx *btcutil.Tx) []byte {
  msgTx := tx.MsgTx()
  opReturnData := []byte{}

x/btccheckpoint/keeper/keeper.go

@@ -55,7 +55,7 @@ func (k Keeper) GetBlockHeight(b wire.BlockHeader) (uint64, error) {
 }
 
 func (k Keeper) GetCheckpointEpoch(c []byte) (uint64, error) {
- return k.checkpointingKeeper.CheckpointValid(c)
+ return k.checkpointingKeeper.CheckpointEpoch(c)
 }
 
 // TODO for now we jsut store raw checkpoint with epoch as key. Ultimatly

x/btccheckpoint/keeper/msg_server.go

@@ -19,11 +19,8 @@ func NewMsgServerImpl(keeper Keeper) types.MsgServer {
  return &msgServer{keeper}
 }
 
-// returns fully assembled rawcheckpoint data and the latest header number of
-// headers provided in the proof
-func (m msgServer) getRawCheckPointAndBtchHeight(proofs []*btcutils.ParsedProof) ([]byte, uint64, error) {
+func (m msgServer) getProofHeight(proofs []*btcutils.ParsedProof) (uint64, error) {
  var latestblock = uint64(0)
- var rawCheckpointData []byte
 
  for _, proof := range proofs {
  // TODO consider interfaces here. As currently this implicity assume that all headers
@@ -33,19 +30,29 @@ func (m msgServer) getRawCheckPointAndBtchHeight(proofs []*btcutils.ParsedProof)
  num, err := m.k.GetBlockHeight(proof.BlockHeader)
 
  if err != nil {
- return nil, 0, err
+ return latestblock, err
  }
 
  // returning hightes block number as checkpoint number as if highest becomes
  // stable then it means older is also stable.
  if num > latestblock {
  latestblock = num
  }
+ }
+
+ return latestblock, nil
+}
 
+// returns fully assembled rawcheckpoint data and the latest header number of
+// headers provided in the proof
+func (m msgServer) getRawCheckPoint(proofs []*btcutils.ParsedProof) []byte {
+ var rawCheckpointData []byte
+
+ for _, proof := range proofs {
  rawCheckpointData = append(rawCheckpointData, proof.OpReturnData...)
  }
 
- return rawCheckpointData, latestblock, nil
+ return rawCheckpointData
 }
 
 // TODO at some point add proper logging of error
@@ -61,7 +68,7 @@ func (m msgServer) InsertBTCSpvProof(ctx context.Context, req *types.MsgInsertBT
 
  // TODO for now we do nothing with processed blockHeight but ultimatly it should
  // be a part of timestamp
- rawCheckPointData, _, err := m.getRawCheckPointAndBtchHeight(proofs)
+ _, err := m.getProofHeight(proofs)
 
  if err != nil {
  return nil, err
@@ -72,6 +79,7 @@ func (m msgServer) InsertBTCSpvProof(ctx context.Context, req *types.MsgInsertBT
  // part of provided block and contains some OP_RETURN data
  // - header is proved to be part of the chain we know about thorugh BTCLightClient
  // Inform checkpointing module about it.
+ rawCheckPointData := m.getRawCheckPoint(proofs)
 
  epochNum, err := m.k.GetCheckpointEpoch(rawCheckPointData)
 

x/btccheckpoint/types/expected_keepers.go

@@ -25,8 +25,7 @@ type BTCLightClientKeeper interface {
 }
 
 type CheckpointingKeeper interface {
- // Function should return epoch of given raw checkpoint or indicate that checkpoint
- // is invalid
- // If chekpoint is valid checkpointing module should store it.
- CheckpointValid(rawCheckpoint []byte) (uint64, error)
+ // CheckpointEpoch should return epoch index it passes all checkpoint validation
+ // and error otherwise
+ CheckpointEpoch(rawCheckpoint []byte) (uint64, error)
 }

x/btccheckpoint/types/mock_keepers.go

@@ -12,6 +12,6 @@ func (mb MockBTCLightClientKeeper) BlockHeight(header wire.BlockHeader) (uint64,
  return uint64(10), nil
 }
 
-func (ck MockCheckpointingKeeper) CheckpointValid(rawCheckpoint []byte) (uint64, error) {
+func (ck MockCheckpointingKeeper) CheckpointEpoch(rawCheckpoint []byte) (uint64, error) {
  return uint64(10), nil
 }

x/btccheckpoint/types/msgs.go

@@ -23,14 +23,14 @@ const (
  expectedProofs = 2
 )
 
-// Parse and Validate provided OP_RETRUN transaction with their proofs.
+// Parse and Validate provided OP_RETURN transaction with their proofs.
 // If even one transaction is invalid error is returned.
 // Returned ParsedProofs are in same order as raw proofs
 // TODO explore possibility of validating that output in second tx is payed by
 // input in the first tx
 func ParseTwoProofs(proofs []*BTCSpvProof, powLimit *big.Int) ([]*btcutils.ParsedProof, error) {
  if len(proofs) != expectedProofs {
- return nil, fmt.Errorf("expected at least two valid op return transactions")
+ return nil, fmt.Errorf("expected at exactly valid op return transactions")
  }
 
  var parsedProofs []*btcutils.ParsedProof
@@ -61,12 +61,12 @@ func (m *MsgInsertBTCSpvProof) ValidateBasic() error {
  }
 
  // TODO get powLimit from some config
- // result of parsed proog is notneeded, drop it
+ // result of parsed proof is not needed, drop it
  // whole parsing stuff is stateless
- _, e := ParseTwoProofs(m.Proofs, btcchaincfg.MainNetParams.PowLimit)
+ _, err := ParseTwoProofs(m.Proofs, btcchaincfg.MainNetParams.PowLimit)
 
- if e != nil {
- return e
+ if err != nil {
+ return err
  }
 
  return nil