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

zoneconcierge: proof that a tx is in a block #234

Merged
merged 4 commits into from
Dec 6, 2022

Conversation

SebastianElvis
Copy link
Member

@SebastianElvis SebastianElvis commented Dec 6, 2022

Fixes BM-301

This PR implements ProofTxInBlock, the proof that a tx is included in a block. It includes the prover and the verifier.

Approach

To get this proof, ZoneConcierge module will need to maintain a Tendermint query client, and direct the client to make queries to itself, similar to BLS signer that submits txs to Babylon itself.

This is the only possible way to get the tx inclusion proof: Cosmos SDK does not provide APIs for querying the underlying Tendermint blockchain. Everything one can query from a Cosmos SDK-based chain is included in interface ABCIApplicationServer defined here. It does not include querying block or tx. This makes sense since Cosmos SDK does not make any assumption on the underlying consensus engine (e.g., one can implement its own blockchain and talks to Cosmos SDK by using the same set of APIs).

Future work

One important task is to have an integration test that runs Babylon and CZ (including their Tendermint nodes), and tests the inclusion of a CZ timestamp. Currently, this PR only tests the inclusion of a tx with a MsgSend msg, and tests the FinalizedChainInfo query with a mocked Tendermint client.

Copy link
Contributor

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

Awesome! Only some minor comments:

app/app.go Outdated
@@ -406,6 +407,10 @@ func NewBabylonApp(
scopedIBCKeeper,
)

tmClient, err := client.NewClientFromNode("tcp://localhost:26657") // create a Tendermint client for ZoneConcierge
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we read the url from config? For example, maybe we can read config from client.toml like this here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! Used the node URI in clientCtx for this 👍

app/app.go Outdated
@@ -406,6 +407,10 @@ func NewBabylonApp(
scopedIBCKeeper,
)

tmClient, err := client.NewClientFromNode("tcp://localhost:26657") // create a Tendermint client for ZoneConcierge
if err != nil {
panic(fmt.Errorf("couldn't get client from nodeURI: %v", err))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not for this case but I would suggest we use %w to wrap an error. See here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Didn't know this usage before!

@@ -92,11 +92,16 @@ func (k Keeper) FinalizedChainInfo(c context.Context, req *types.QueryFinalizedC
bestSubmissionKey := ed.Key[0]

// TODO: construct inclusion proofs
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We still get 3 inclusion proofs to implement after this PR: ProofBlockInEpoch, ProofEpochEnded and ProofEpochSubmitted

@@ -70,3 +73,8 @@ type EpochingKeeper interface {
GetHistoricalEpoch(ctx sdk.Context, epochNumber uint64) (*epochingtypes.Epoch, error)
GetEpoch(ctx sdk.Context) *epochingtypes.Epoch
}

// TMClient is a Tendermint that allows to query tx inclusion proofs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TMClient is a Tendermint that allows to query tx inclusion proofs
// TMClient is a Tendermint client that allows to query tx inclusion proofs

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

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

Looks good!

)

func (k Keeper) ProveTxInBlock(ctx sdk.Context, txHash []byte) (*tmproto.TxProof, error) {
if len(txHash) != sha256.Size {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a custom type can be created for the transaction hash. I would also expect that tendermint or cosmos would have a type for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, Cosmos SDK and Tendermint do not define a new type of txhash, but use hex string or bytes for it. See here and here

@SebastianElvis SebastianElvis merged commit d20d09f into dev Dec 6, 2022
@SebastianElvis SebastianElvis deleted the zoneconcierge-proof-tx-in-block branch December 6, 2022 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants