Add qgb data commitment tests #240
Conversation
| @@ -119,3 +119,48 @@ func TestMsgValsetConfirm(t *testing.T) { | |||
| _, err = h(ctx, msg) | |||
| require.NoError(t, err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This will be fixed in a subsequent PR after I write an issue about a confusion we have with the validator address for data commitments
| commitment string, | ||
| ) (confirms []types.MsgDataCommitmentConfirm) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| iterator := store.Iterator(nil, nil) // Can we make this faster? |
There was a problem hiding this comment.
For now, this is iterating over the whole store and skipping elements it cannot unmarshall.
Any idea on how to make this faster ? Or maybe have some sort of indexing...
There was a problem hiding this comment.
yeah, we can index by adding different prefixes
https://github.com/celestiaorg/cosmos-sdk/blob/568e61059a5275d63fae7980e196ff7b6ce07426/x/auth/types/keys.go#L22-L23
my understanding is if we wanted to index by nonce and MsgDataCommitmentConfirms, then we would define each prefix ahead of time, then append them when storing. something along the lines of MsgDataCommitmentConfirmKey + nonce + commitment.
let's obviously not tackle that in this PR, but we should probably discuss this further in its own issue and call #241
| if err != nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Skipping if the item cannot be unmarshalled (not the correct interface)
|
@evan-forbes Please take a look at this when you have time. Thanks. |
| @@ -119,3 +119,48 @@ func TestMsgValsetConfirm(t *testing.T) { | |||
| _, err = h(ctx, msg) | |||
| require.NoError(t, err) | |||
| } | |||
|
|
|||
| commitment string, | ||
| ) (confirms []types.MsgDataCommitmentConfirm) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| iterator := store.Iterator(nil, nil) // Can we make this faster? |
There was a problem hiding this comment.
yeah, we can index by adding different prefixes
https://github.com/celestiaorg/cosmos-sdk/blob/568e61059a5275d63fae7980e196ff7b6ce07426/x/auth/types/keys.go#L22-L23
my understanding is if we wanted to index by nonce and MsgDataCommitmentConfirms, then we would define each prefix ahead of time, then append them when storing. something along the lines of MsgDataCommitmentConfirmKey + nonce + commitment.
let's obviously not tackle that in this PR, but we should probably discuss this further in its own issue and call #241
|
@evan-forbes I have a final test almost ready. |
|
merging this is fine I think as its targeted very specifically at data commitments, and then having a separate PR for the integration test makes sense imho. up to you tho |
|
Sounds good. Please merge. Thanks |
* fixes data commitment keeper iterator * adds data commitment by commitment query * cosmetics * adds data commitment queries tests * cosmetics * adds data commitment message validation tests * adds commented msg data commitment confirm test
Description
Adds QGB data commitment tests
closes: #238