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

feat(tm2): Add Specific Block Height query Feature to ABCI Queries #2722

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
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
18 changes: 9 additions & 9 deletions tm2/pkg/bft/rpc/client/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,16 @@ func (b *RPCBatch) ABCIQuery(path string, data []byte) error {
}

func (b *RPCBatch) ABCIQueryWithOptions(path string, data []byte, opts ABCIQueryOptions) error {
params := map[string]any{
"path": path,
"data": data,
"prove": opts.Prove,
}
if opts.Height > 0 {
params["height"] = opts.Height
}
// Prepare the RPC request
request, err := newRequest(
abciQueryMethod,
map[string]any{
"path": path,
"data": data,
"height": opts.Height,
"prove": opts.Prove,
},
)
request, err := newRequest(abciQueryMethod, params)
if err != nil {
return fmt.Errorf("unable to create request, %w", err)
}
Expand Down
18 changes: 18 additions & 0 deletions tm2/pkg/bft/rpc/client/batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,24 @@ func TestRPCBatch_Endpoints(t *testing.T) {
return castResult
},
},
{
abciQueryMethod,
&ctypes.ResultABCIQuery{
Response: abci.ResponseQuery{
Value: []byte("dummy"),
Height: 10,
},
},
func(batch *RPCBatch) {
require.NoError(t, batch.ABCIQueryWithOptions("path", []byte("dummy"), ABCIQueryOptions{Height: 10, Prove: false}))
},
func(result any) any {
castResult, ok := result.(*ctypes.ResultABCIQuery)
require.True(t, ok)
assert.Equal(t, int64(10), castResult.Response.Height)
return castResult
},
},
{
broadcastTxCommitMethod,
&ctypes.ResultBroadcastTxCommit{
Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/bft/rpc/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ func (c *RPCClient) ABCIQueryWithOptions(path string, data []byte, opts ABCIQuer
map[string]any{
"path": path,
"data": data,
"height": opts.Height,
"prove": opts.Prove,
"height": opts.Height,
},
)
}
Expand Down
16 changes: 16 additions & 0 deletions tm2/pkg/bft/rpc/client/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,22 @@ func TestRPCClient_E2E_Endpoints(t *testing.T) {
assert.Equal(t, expectedResult, result)
},
},
{
abciQueryMethod,
&ctypes.ResultABCIQuery{
Response: abci.ResponseQuery{
Value: []byte("dummy"),
Height: 10,
},
},
func(client *RPCClient, expectedResult any) {
result, err := client.ABCIQueryWithOptions("path", []byte("dummy"), ABCIQueryOptions{Height: 10, Prove: false})
require.NoError(t, err)

assert.Equal(t, expectedResult, result)
assert.Equal(t, int64(10), result.Response.Height)
},
},
{
broadcastTxCommitMethod,
&ctypes.ResultBroadcastTxCommit{
Expand Down
1 change: 0 additions & 1 deletion tm2/pkg/bft/rpc/client/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ type ABCIClient interface {
ABCIQuery(path string, data []byte) (*ctypes.ResultABCIQuery, error)
ABCIQueryWithOptions(path string, data []byte,
opts ABCIQueryOptions) (*ctypes.ResultABCIQuery, error)

// Writing to abci app
BroadcastTxCommit(tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error)
BroadcastTxAsync(tx types.Tx) (*ctypes.ResultBroadcastTx, error)
Expand Down
22 changes: 17 additions & 5 deletions tm2/pkg/bft/rpc/core/abci.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
package core

import (
"fmt"

abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
ctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/core/types"
rpctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types"
)

// Query the application for some information.
// Query the application for some information. It allows querying the
// application's state at a specific height.
//
// ```shell
// curl 'localhost:26657/abci_query?path=""&data="abcd"&prove=false'
// curl 'localhost:26657/abci_query?path=""&data="abcd"&height=10&prove=false'
// ```
//
// ```go
Expand All @@ -21,7 +24,7 @@ import (
// }
//
// defer client.Stop()
// result, err := client.ABCIQuery("", "abcd", true)
// result, err := client.ABCIQuery("", "abcd", 10, true)
// ```
//
// > The above command returns JSON structured like this:
Expand All @@ -33,7 +36,7 @@ import (
// "result": {
// "response": {
// "log": "exists",
// "height": "0",
// "height": "10",
// "proof": "010114FED0DAD959F36091AD761C922ABA3CBF1D8349990101020103011406AA2262E2F448242DF2C2607C3CDC705313EE3B0001149D16177BC71E445476174622EA559715C293740C",
// "value": "61626364",
// "key": "61626364",
Expand All @@ -56,6 +59,15 @@ import (
// | height | int64 | 0 | false | Height (0 means latest) |
// | prove | bool | false | false | Includes proof if true |
func ABCIQuery(ctx *rpctypes.Context, path string, data []byte, height int64, prove bool) (*ctypes.ResultABCIQuery, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a small unit test that verifies the errors?

I know our RPC testing suite is super janky, but it shouldn't be too much of a hassle to write a few cases

Copy link
Member

Choose a reason for hiding this comment

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

I promise refactoring this testing suite is on my bucket list

if height < 0 {
return nil, fmt.Errorf("height cannot be negative: %d", height)
}
currentHeight := blockStore.Height()

if height > currentHeight {
return nil, fmt.Errorf("requested height %d is in the future (latest height is %d)", height, currentHeight)
}

resQuery, err := proxyAppQuery.QuerySync(abci.RequestQuery{
Path: path,
Data: data,
Expand All @@ -65,7 +77,7 @@ func ABCIQuery(ctx *rpctypes.Context, path string, data []byte, height int64, pr
if err != nil {
return nil, err
}
logger.Info("ABCIQuery", "path", path, "data", data, "result", resQuery)

return &ctypes.ResultABCIQuery{Response: resQuery}, nil
}

Expand Down
101 changes: 101 additions & 0 deletions tm2/pkg/bft/rpc/core/abci_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package core

import (
"testing"

"github.com/stretchr/testify/assert"

abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
ctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/core/types"
rpctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types"
)

func TestABCIQuery(t *testing.T) {

Check failure on line 13 in tm2/pkg/bft/rpc/core/abci_test.go

View workflow job for this annotation

GitHub Actions / Run Main / Go Linter / lint

TestABCIQuery should use t.Cleanup instead of defer (tparallel)
t.Parallel()

origBlockStore, origProxyAppQuery := blockStore, proxyAppQuery
defer func() {
blockStore, proxyAppQuery = origBlockStore, origProxyAppQuery
}()

tests := []struct {
name string
path string
data []byte
height int64
prove bool
mockHeight int64
mockQueryResp *abci.ResponseQuery
mockQueryErr error
expectedResult *ctypes.ResultABCIQuery
expectedError string
}{
{
name: "valid query",
path: "/a/b/c",
data: []byte("data"),
height: 10,
prove: false,
mockHeight: 20,
mockQueryResp: &abci.ResponseQuery{
Key: []byte("key"),
Value: []byte("result"),
Height: 10,
},
expectedResult: &ctypes.ResultABCIQuery{
Response: abci.ResponseQuery{
Key: []byte("key"),
Value: []byte("result"),
Height: 10,
},
},
},
{
name: "negative height",
height: -1,
mockHeight: 20,
expectedResult: nil,
expectedError: "height cannot be negative",
},
{
name: "future height",
height: 30,
mockHeight: 20,
expectedResult: nil,
expectedError: "requested height 30 is in the future (latest height is 20)",
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

mockBS := &mockBlockStore{
heightFn: func() int64 { return tt.mockHeight },
}
blockStore = mockBS

mockPAQ := &mockProxyAppQuery{
queryFn: func(req abci.RequestQuery) (abci.ResponseQuery, error) {
if tt.mockQueryResp != nil {
return *tt.mockQueryResp, tt.mockQueryErr
}
return abci.ResponseQuery{}, tt.mockQueryErr
},
}
proxyAppQuery = mockPAQ

result, err := ABCIQuery(&rpctypes.Context{}, tt.path, tt.data, tt.height, tt.prove)

if tt.expectedError != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.expectedError)
assert.Nil(t, result)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expectedResult, result)
}
})
}
}
28 changes: 27 additions & 1 deletion tm2/pkg/bft/rpc/core/mock_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package core

import "github.com/gnolang/gno/tm2/pkg/bft/types"
import (
abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
"github.com/gnolang/gno/tm2/pkg/bft/types"
)

type (
heightDelegate func() int64
Expand Down Expand Up @@ -76,3 +79,26 @@ func (m *mockBlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet
m.saveBlockFn(block, blockParts, seenCommit)
}
}

type mockProxyAppQuery struct {
queryFn func(req abci.RequestQuery) (abci.ResponseQuery, error)
}

func (m *mockProxyAppQuery) QuerySync(req abci.RequestQuery) (abci.ResponseQuery, error) {
if m.queryFn != nil {
return m.queryFn(req)
}
return abci.ResponseQuery{}, nil
}

func (m *mockProxyAppQuery) EchoSync(msg string) (abci.ResponseEcho, error) {
return abci.ResponseEcho{}, nil
}

func (m *mockProxyAppQuery) InfoSync(req abci.RequestInfo) (abci.ResponseInfo, error) {
return abci.ResponseInfo{}, nil
}

func (m *mockProxyAppQuery) Error() error {
return nil
}
27 changes: 23 additions & 4 deletions tm2/pkg/crypto/keys/client/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
type QueryCfg struct {
RootCfg *BaseCfg

Data string
Path string
Data string
Path string
Height int64
Prove bool
}

func NewQueryCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command {
Expand Down Expand Up @@ -42,6 +44,18 @@
"",
"query data bytes",
)
fs.Int64Var(
&c.Height,
"height",
0,
"height to query (0 means latest)",
)
fs.BoolVar(
&c.Prove,
"prove",
false,
"include proofs of the transactions inclusion in the block",
)
}

func execQuery(cfg *QueryCfg, args []string, io commands.IO) error {
Expand Down Expand Up @@ -69,6 +83,11 @@
io.Printf("height: %d\ndata: %s\n",
height,
string(resdata))
if cfg.Prove {
io.Printf("proof: %x\n",
qres.Response.Proof,
)

Check warning on line 89 in tm2/pkg/crypto/keys/client/query.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/client/query.go#L86-L89

Added lines #L86 - L89 were not covered by tests
}
return nil
}

Expand All @@ -80,8 +99,8 @@

data := []byte(cfg.Data)
opts2 := client.ABCIQueryOptions{
// Height: height, XXX
// Prove: false, XXX
Height: cfg.Height,
Prove: cfg.Prove,

Check warning on line 103 in tm2/pkg/crypto/keys/client/query.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/client/query.go#L102-L103

Added lines #L102 - L103 were not covered by tests
}
cli, err := client.NewHTTPClient(remote)
if err != nil {
Expand Down
Loading
Loading