Skip to content
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

Btccheckpoint oracle schema and processing #43

Merged
merged 3 commits into from
Jul 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions proto/babylon/btccheckpoint/btccheckpoint.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
syntax = "proto3";
package babylon.btccheckpoint;

import "gogoproto/gogo.proto";

option go_package = "github.com/babylonchain/babylon/x/btccheckpoint/types";

// Each provided OP_RETURN transaction can be idendtified by hash of block in
// which transaction was included and transaction index in the block
message TransactionKey {
uint32 index = 1;
bytes hash = 2 [
(gogoproto.customtype) = "github.com/babylonchain/babylon/types.BTCHeaderHashBytes"
];
}

// Checkpoint can be composed from multiple transactions, so to identify whole
// submission we need list of transaction keys.
// Each submission can generally be identified by this list of (txIdx, blockHash)
// tuples.
// Note: this could possibly be optimized as if transactions were in one block
// they would have the same block hash and different indexes, but each blockhash
// is only 33 (1 byte for prefix encoding and 32 byte hash), so there should
// be other strong arguments for this optimization
message SubmissionKey {
repeated TransactionKey key = 1;
}

// TODO: Determine if we should keep any block number or depth info.
// On one hand it may be usefull to determine if block is stable or not, on other
// depth/block number info, without context (i.e info about chain) is pretty useless
// and blockshash in enough to retrieve is from lightclient
message SubmissionData {
// TODO: this could probably be better typed
// Address of submitter of given checkpoint. Required to payup the reward to
// submitter of given checkpoint
bytes submitter = 1;
// Required to recover address of sender of btc transction to payup the reward.
// TODO: Maybe it is worth recovering senders while processing the InsertProof
// message, and store only those. Another point is that it is not that simple
// to recover sender of btc tx.
repeated bytes btctransaction = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add a custom type for this. If we follow the "fortress" approach which is defined for BTCHeaderBytes and BTCHeaderHashBytes, then verification would happen in only one place, while we could efficiently display it as a hex.

Copy link
Collaborator Author

@KonradStaniec KonradStaniec Jul 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I am not sure this thing will ultimately be here. For now I keep it here to extract btc sender later, but I am not sure it is such each task to do, so maybe some other approach will be need (like having btc sender in the message directly ?).

That's why for now I would leave it like that and add the approach you mention in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fishermanymc suggested once to put the Babylon account to be rewarded into the OP_RETURN, mainly to do away with having to register the BTC->BBL mapping up front. It felt to me like that would be using a limited resource for something we can get around by other means. But it's an option.

}

// Data stored in db and indexed by epoch number
// TODO: Add btc blockheight at epooch end, when adding hadnling of epoching callbacks
message EpochData {
// List of all received checkpoints during this epoch, sorted by order of
// submission.
repeated SubmissionKey key = 1;
}

21 changes: 14 additions & 7 deletions x/btccheckpoint/btcutils/btcutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
)

const (
Expand All @@ -20,12 +19,17 @@ const (

// Parsed proof represent semantically valid:
// - Bitcoin Header
// - Bitcoin Header hash
// - Bitcoin Transaction
// - Bitcoin Transaction index in block
// - Non-empty OpReturnData
type ParsedProof struct {
BlockHeader wire.BlockHeader
Transaction *btcutil.Tx
OpReturnData []byte
// keeping header hash to avoid recomputing it everytime
BlockHash types.BTCHeaderHashBytes
Transaction *btcutil.Tx
TransactionBytes []byte
TransactionIdx uint32
OpReturnData []byte
}

// Concatenates and double hashes two provided inputs
Expand Down Expand Up @@ -147,10 +151,13 @@ func ParseProof(
return nil, fmt.Errorf("provided transaction should provide op return data")
}

bh := header.BlockHash()
parsedProof := &ParsedProof{
BlockHeader: *header,
Transaction: tx,
OpReturnData: opReturnData,
BlockHash: types.NewBTCHeaderHashBytesFromChainhash(&bh),
Transaction: tx,
TransactionBytes: btcTransaction,
TransactionIdx: transactionIndex,
OpReturnData: opReturnData,
}

return parsedProof, nil
Expand Down
47 changes: 35 additions & 12 deletions x/btccheckpoint/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package keeper
import (
"fmt"

"github.com/btcsuite/btcd/wire"
"github.com/tendermint/tendermint/libs/log"

btypes "github.com/babylonchain/babylon/types"
"github.com/babylonchain/babylon/x/btccheckpoint/types"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -50,26 +50,49 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName))
}

func (k Keeper) GetBlockHeight(b wire.BlockHeader) (uint64, error) {
func (k Keeper) GetBlockHeight(b btypes.BTCHeaderHashBytes) (uint64, error) {
return k.btcLightClientKeeper.BlockHeight(b)
}

func (k Keeper) IsAncestor(parentHash btypes.BTCHeaderHashBytes, childHash btypes.BTCHeaderHashBytes) (bool, error) {
return k.btcLightClientKeeper.IsAncestor(parentHash, childHash)
}

func (k Keeper) GetCheckpointEpoch(c []byte) (uint64, error) {
return k.checkpointingKeeper.CheckpointEpoch(c)
}

// TODO for now we jsut store raw checkpoint with epoch as key. Ultimatly
// we should store checkpoint with some timestamp info, or even do not store
// checkpoint itelf but its status
func (k Keeper) StoreCheckpoint(ctx sdk.Context, e uint64, c []byte) {
func (k Keeper) SubmissionExists(ctx sdk.Context, sk types.SubmissionKey) bool {
store := ctx.KVStore(k.storeKey)
kBytes := k.cdc.MustMarshal(&sk)
return store.Has(kBytes)
}

// Return epoch data for given epoch, if there is not epoch data yet returns nil
func (k Keeper) GetEpochData(ctx sdk.Context, e uint64) *types.EpochData {
store := ctx.KVStore(k.storeKey)
bytes := store.Get(types.GetEpochIndexKey(e))

if len(bytes) == 0 {
return nil
}

ed := &types.EpochData{}
k.cdc.MustUnmarshal(bytes, ed)
return ed

}

func (k Keeper) SaveEpochData(ctx sdk.Context, e uint64, ed *types.EpochData) {
store := ctx.KVStore(k.storeKey)
key := sdk.Uint64ToBigEndian(e)
store.Set(key, c)
ek := types.GetEpochIndexKey(e)
eb := k.cdc.MustMarshal(ed)
store.Set(ek, eb)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the epoch data is already set? Is it ok to override it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that's the idea, that the list of submission keys gets expanded.

}

// TODO just return checkpoint if exists
func (k Keeper) GetCheckpoint(ctx sdk.Context, e uint64) []byte {
func (k Keeper) SaveSubmission(ctx sdk.Context, sk types.SubmissionKey, sd types.SubmissionData) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if instead of having a separate SaveEpochData and SaveSubmission there should be one AddSubmission(epochNumber, submission) that calculates the keys on its own - it could even get the epoch number from the submission but I'm not sure what the state of your thinking is about who knows what about the op_return content.

That way I could not use public methods of the keeper to:

  • set submission data with the wrong key
  • insert submission data that doesn't get recorded in the epoch
  • insert submission keys in the epoch that don't exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking about it , and currently have method checkAndSaveEpochData in msg_server with todo // TODO maybe move it to keeper
but I wanted to consider it after pr which will use the hooks from epoching and lightclient to manipulate submission data in someway. I imagine that SaveSubmission will come in handy on its own then but we will see.

store := ctx.KVStore(k.storeKey)
key := sdk.Uint64ToBigEndian(e)
return store.Get(key)
kBytes := k.cdc.MustMarshal(&sk)
sBytes := k.cdc.MustMarshal(&sd)
store.Set(kBytes, sBytes)
}
Loading