From 9d5c4cd77849a5df5f633bee30a4b68247912a71 Mon Sep 17 00:00:00 2001 From: Jason Paulos Date: Thu, 29 Aug 2024 14:47:05 -0400 Subject: [PATCH] REST API: Fix `LedgerStateDelta` JSON encoding (#6106) Co-authored-by: Ashy5000 --- daemon/algod/api/client/restClient.go | 23 +++++++++--- daemon/algod/api/server/v2/handlers.go | 32 +++++++++++++--- data/transactions/transaction.go | 6 +-- libgoal/libgoal.go | 5 ++- test/e2e-go/features/devmode/devmode_test.go | 27 +++++++++++--- .../features/followernode/syncDeltas_test.go | 37 ++++++++++++++++++- 6 files changed, 107 insertions(+), 23 deletions(-) diff --git a/daemon/algod/api/client/restClient.go b/daemon/algod/api/client/restClient.go index 33b8da8ee1..a3dc4b769e 100644 --- a/daemon/algod/api/client/restClient.go +++ b/daemon/algod/api/client/restClient.go @@ -30,11 +30,14 @@ import ( "github.com/google/go-querystring/query" "github.com/algorand/go-algorand/crypto" + v2 "github.com/algorand/go-algorand/daemon/algod/api/server/v2" "github.com/algorand/go-algorand/daemon/algod/api/server/v2/generated/model" "github.com/algorand/go-algorand/daemon/algod/api/spec/common" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/transactions" "github.com/algorand/go-algorand/data/transactions/logic" + "github.com/algorand/go-algorand/ledger/eval" + "github.com/algorand/go-algorand/ledger/ledgercore" "github.com/algorand/go-algorand/protocol" ) @@ -229,7 +232,7 @@ func (client RestClient) submitForm( } if decodeJSON { - dec := json.NewDecoder(resp.Body) + dec := protocol.NewJSONDecoder(resp.Body) return dec.Decode(&response) } @@ -559,7 +562,9 @@ func (client RestClient) SendRawTransactionGroup(txgroup []transactions.SignedTx } // Block gets the block info for the given round -func (client RestClient) Block(round uint64) (response model.BlockResponse, err error) { +func (client RestClient) Block(round uint64) (response v2.BlockResponseJSON, err error) { + // Note: this endpoint gets the Block as JSON, meaning some string fields with non-UTF-8 data will lose + // information. Msgpack should be used instead if this becomes a problem. err = client.get(&response, fmt.Sprintf("/v2/blocks/%d", round), nil) return } @@ -767,19 +772,27 @@ func (client RestClient) GetSyncRound() (response model.GetSyncRoundResponse, er } // GetLedgerStateDelta retrieves the ledger state delta for the round -func (client RestClient) GetLedgerStateDelta(round uint64) (response model.LedgerStateDeltaResponse, err error) { +func (client RestClient) GetLedgerStateDelta(round uint64) (response ledgercore.StateDelta, err error) { + // Note: this endpoint gets the StateDelta as JSON, meaning some string fields with non-UTF-8 data will lose + // information. Msgpack should be used instead if this becomes a problem. err = client.get(&response, fmt.Sprintf("/v2/deltas/%d", round), nil) return } // GetLedgerStateDeltaForTransactionGroup retrieves the ledger state delta for the txn group specified by the id -func (client RestClient) GetLedgerStateDeltaForTransactionGroup(id string) (response model.LedgerStateDeltaForTransactionGroupResponse, err error) { +func (client RestClient) GetLedgerStateDeltaForTransactionGroup(id string) (response eval.StateDeltaSubset, err error) { + // Note: this endpoint gets the StateDelta as JSON, meaning some string fields with non-UTF-8 data will lose + // information. Msgpack should be used instead if this becomes a problem. err = client.get(&response, fmt.Sprintf("/v2/deltas/txn/group/%s", id), nil) return } // GetTransactionGroupLedgerStateDeltasForRound retrieves the ledger state deltas for the txn groups in the specified round -func (client RestClient) GetTransactionGroupLedgerStateDeltasForRound(round uint64) (response model.TransactionGroupLedgerStateDeltasForRoundResponse, err error) { +func (client RestClient) GetTransactionGroupLedgerStateDeltasForRound(round uint64) (response struct { + Deltas []eval.TxnGroupDeltaWithIds +}, err error) { + // Note: this endpoint gets the StateDelta as JSON, meaning some string fields with non-UTF-8 data will lose + // information. Msgpack should be used instead if this becomes a problem. err = client.get(&response, fmt.Sprintf("/v2/deltas/%d/txn/group", round), nil) return } diff --git a/daemon/algod/api/server/v2/handlers.go b/daemon/algod/api/server/v2/handlers.go index b638825ec4..9c5da32b3d 100644 --- a/daemon/algod/api/server/v2/handlers.go +++ b/daemon/algod/api/server/v2/handlers.go @@ -673,6 +673,11 @@ func (v2 *Handlers) AccountApplicationInformation(ctx echo.Context, address stri return ctx.JSON(http.StatusOK, response) } +// BlockResponseJSON is used to embed the block in JSON responses. +type BlockResponseJSON struct { + Block bookkeeping.Block `codec:"block"` +} + // GetBlock gets the block for the given round. // (GET /v2/blocks/{round}) func (v2 *Handlers) GetBlock(ctx echo.Context, round uint64, params model.GetBlockParams) error { @@ -709,9 +714,7 @@ func (v2 *Handlers) GetBlock(ctx echo.Context, round uint64, params model.GetBlo } // Encoding wasn't working well without embedding "real" objects. - response := struct { - Block bookkeeping.Block `codec:"block"` - }{ + response := BlockResponseJSON{ Block: block, } @@ -839,7 +842,7 @@ func (v2 *Handlers) GetBlockHash(ctx echo.Context, round uint64) error { // (GET /v2/blocks/{round}/transactions/{txid}/proof) func (v2 *Handlers) GetTransactionProof(ctx echo.Context, round uint64, txid string, params model.GetTransactionProofParams) error { var txID transactions.Txid - err := txID.UnmarshalText([]byte(txid)) + err := txID.FromString(txid) if err != nil { return badRequest(ctx, err, errNoValidTxnSpecified, v2.Log) } @@ -1432,6 +1435,11 @@ func (v2 *Handlers) GetLedgerStateDelta(ctx echo.Context, round uint64, params m if err != nil { return notFound(ctx, err, fmt.Sprintf(errFailedRetrievingStateDelta, err), v2.Log) } + if handle == protocol.JSONStrictHandle { + // Zero out the Txleases map since it cannot be represented in JSON, as it is a map with an + // object key. + sDelta.Txleases = nil + } data, err := encode(handle, sDelta) if err != nil { return internalError(ctx, err, errFailedToEncodeResponse, v2.Log) @@ -1501,8 +1509,8 @@ func (v2 *Handlers) PendingTransactionInformation(ctx echo.Context, txid string, } txID := transactions.Txid{} - if err := txID.UnmarshalText([]byte(txid)); err != nil { - return badRequest(ctx, err, errNoValidTxnSpecified, v2.Log) + if err0 := txID.FromString(txid); err0 != nil { + return badRequest(ctx, err0, errNoValidTxnSpecified, v2.Log) } txn, ok := v2.Node.GetPendingTransaction(txID) @@ -2022,6 +2030,11 @@ func (v2 *Handlers) GetLedgerStateDeltaForTransactionGroup(ctx echo.Context, id if err != nil { return notFound(ctx, err, fmt.Sprintf(errFailedRetrievingStateDelta, err), v2.Log) } + if handle == protocol.JSONStrictHandle { + // Zero out the Txleases map since it cannot be represented in JSON, as it is a map with an + // object key. + delta.Txleases = nil + } data, err := encode(handle, delta) if err != nil { return internalError(ctx, err, errFailedToEncodeResponse, v2.Log) @@ -2044,6 +2057,13 @@ func (v2 *Handlers) GetTransactionGroupLedgerStateDeltasForRound(ctx echo.Contex if err != nil { return notFound(ctx, err, fmt.Sprintf(errFailedRetrievingStateDelta, err), v2.Log) } + if handle == protocol.JSONStrictHandle { + // Zero out the Txleases map since it cannot be represented in JSON, as it is a map with an + // object key. + for i := range deltas { + deltas[i].Delta.Txleases = nil + } + } response := struct { Deltas []eval.TxnGroupDeltaWithIds }{ diff --git a/data/transactions/transaction.go b/data/transactions/transaction.go index 06ae38c0d6..4a6d5b6603 100644 --- a/data/transactions/transaction.go +++ b/data/transactions/transaction.go @@ -38,9 +38,9 @@ func (txid Txid) String() string { return fmt.Sprintf("%v", crypto.Digest(txid)) } -// UnmarshalText initializes the Address from an array of bytes. -func (txid *Txid) UnmarshalText(text []byte) error { - d, err := crypto.DigestFromString(string(text)) +// FromString initializes the Txid from a string +func (txid *Txid) FromString(text string) error { + d, err := crypto.DigestFromString(text) *txid = Txid(d) return err } diff --git a/libgoal/libgoal.go b/libgoal/libgoal.go index e72b0588d6..f3f1c67192 100644 --- a/libgoal/libgoal.go +++ b/libgoal/libgoal.go @@ -27,6 +27,7 @@ import ( algodclient "github.com/algorand/go-algorand/daemon/algod/api/client" v2 "github.com/algorand/go-algorand/daemon/algod/api/server/v2" kmdclient "github.com/algorand/go-algorand/daemon/kmd/client" + "github.com/algorand/go-algorand/ledger/ledgercore" "github.com/algorand/go-algorand/rpcs" "github.com/algorand/go-algorand/config" @@ -819,7 +820,7 @@ func (c *Client) ParsedPendingTransaction(txid string) (txn v2.PreEncodedTxInfo, } // Block takes a round and returns its block -func (c *Client) Block(round uint64) (resp model.BlockResponse, err error) { +func (c *Client) Block(round uint64) (resp v2.BlockResponseJSON, err error) { algod, err := c.ensureAlgodClient() if err == nil { resp, err = algod.Block(round) @@ -1341,7 +1342,7 @@ func (c *Client) GetSyncRound() (rep model.GetSyncRoundResponse, err error) { } // GetLedgerStateDelta gets the LedgerStateDelta on a node w/ EnableFollowMode -func (c *Client) GetLedgerStateDelta(round uint64) (rep model.LedgerStateDeltaResponse, err error) { +func (c *Client) GetLedgerStateDelta(round uint64) (rep ledgercore.StateDelta, err error) { algod, err := c.ensureAlgodClient() if err == nil { return algod.GetLedgerStateDelta(round) diff --git a/test/e2e-go/features/devmode/devmode_test.go b/test/e2e-go/features/devmode/devmode_test.go index 525a6cacbd..bfb6a889ab 100644 --- a/test/e2e-go/features/devmode/devmode_test.go +++ b/test/e2e-go/features/devmode/devmode_test.go @@ -52,7 +52,7 @@ func testDevMode(t *testing.T, version protocol.ConsensusVersion) { firstRound := *txn.ConfirmedRound + 1 blk, err := fixture.AlgodClient.Block(*txn.ConfirmedRound) require.NoError(t, err) - seconds := int64(blk.Block["ts"].(float64)) + seconds := blk.Block.TimeStamp prevTime := time.Unix(seconds, 0) // Set Block timestamp offset to test that consecutive txns properly get their block time set const blkOffset = uint64(1_000_000) @@ -70,7 +70,7 @@ func testDevMode(t *testing.T, version protocol.ConsensusVersion) { require.Equal(t, round-1, uint64(txn.Txn.Txn.FirstValid)) newBlk, err := fixture.AlgodClient.Block(round) require.NoError(t, err) - newBlkSeconds := int64(newBlk.Block["ts"].(float64)) + newBlkSeconds := newBlk.Block.TimeStamp currTime := time.Unix(newBlkSeconds, 0) require.Equal(t, currTime, prevTime.Add(1_000_000*time.Second)) prevTime = currTime @@ -93,7 +93,18 @@ func testTxnGroupDeltasDevMode(t *testing.T, version protocol.ConsensusVersion) require.NoError(t, err) key := crypto.GenerateSignatureSecrets(crypto.Seed{}) receiver := basics.Address(key.SignatureVerifier) - txn := fixture.SendMoneyAndWait(0, 100000, 1000, sender.Address, receiver.String(), "") + + status, err := fixture.AlgodClient.Status() + require.NoError(t, err) + curRound := status.LastRound + + wh, err := fixture.LibGoalClient.GetUnencryptedWalletHandle() + require.NoError(t, err) + + fundingTx, err := fixture.LibGoalClient.SendPaymentFromWalletWithLease(wh, nil, sender.Address, receiver.String(), 1000, 100000, nil, "", [32]byte{1, 2, 3}, basics.Round(curRound).SubSaturate(1), 0) + require.NoError(t, err) + txn, err := fixture.WaitForConfirmedTxn(curRound+uint64(5), fundingTx.ID().String()) + require.NoError(t, err) require.NotNil(t, txn.ConfirmedRound) _, err = fixture.AlgodClient.Block(*txn.ConfirmedRound) require.NoError(t, err) @@ -101,16 +112,20 @@ func testTxnGroupDeltasDevMode(t *testing.T, version protocol.ConsensusVersion) // Test GetLedgerStateDeltaForTransactionGroup and verify the response contains a delta txngroupResponse, err := fixture.AlgodClient.GetLedgerStateDeltaForTransactionGroup(txn.Txn.ID().String()) require.NoError(t, err) - require.True(t, len(txngroupResponse) > 0) + require.NotZero(t, txngroupResponse) // Test GetTransactionGroupLedgerStateDeltasForRound and verify the response contains the delta for our txn roundResponse, err := fixture.AlgodClient.GetTransactionGroupLedgerStateDeltasForRound(1) require.NoError(t, err) require.Equal(t, len(roundResponse.Deltas), 1) groupDelta := roundResponse.Deltas[0] - require.Equal(t, 1, len(groupDelta.Ids)) + require.Len(t, groupDelta.Ids, 1) require.Equal(t, groupDelta.Ids[0], txn.Txn.ID().String()) // Assert that the TxIDs field across both endpoint responses is the same - require.Equal(t, txngroupResponse["Txids"], groupDelta.Delta["Txids"]) + require.Equal(t, txngroupResponse.Txids, groupDelta.Delta.Txids) + + // Txleases should always be nil for JSON responses + require.Nil(t, txngroupResponse.Txleases) + require.Nil(t, groupDelta.Delta.Txleases) } diff --git a/test/e2e-go/features/followernode/syncDeltas_test.go b/test/e2e-go/features/followernode/syncDeltas_test.go index b404a2a5ef..af27c7dda7 100644 --- a/test/e2e-go/features/followernode/syncDeltas_test.go +++ b/test/e2e-go/features/followernode/syncDeltas_test.go @@ -22,6 +22,9 @@ import ( "github.com/stretchr/testify/require" + "github.com/algorand/go-algorand/data/basics" + "github.com/algorand/go-algorand/data/transactions" + "github.com/algorand/go-algorand/ledger/ledgercore" "github.com/algorand/go-algorand/test/framework/fixtures" "github.com/algorand/go-algorand/test/partitiontest" ) @@ -52,6 +55,23 @@ func TestBasicSyncMode(t *testing.T) { nc, err := fixture.GetNodeController("Primary") a.NoError(err) + sender, err := fixture.GetRichestAccount() + require.NoError(t, err) + + status, err := fixture.AlgodClient.Status() + require.NoError(t, err) + curRound := status.LastRound + + wh, err := fixture.LibGoalClient.GetUnencryptedWalletHandle() + require.NoError(t, err) + + fundingTx, err := fixture.LibGoalClient.SendPaymentFromWalletWithLease(wh, nil, sender.Address, sender.Address, 0, 0, nil, "", [32]byte{1, 2, 3}, basics.Round(curRound).SubSaturate(1), 0) + require.NoError(t, err) + txn, err := fixture.WaitForConfirmedTxn(5, fundingTx.ID().String()) + require.NoError(t, err) + + require.LessOrEqual(t, *txn.ConfirmedRound, uint64(5), "Transaction should be confirmed in the first 5 rounds") + // Let the network make some progress waitForRound := uint64(5) err = fixture.ClientWaitForRoundWithTimeout(fixture.GetAlgodClientForController(nc), waitForRound) @@ -73,7 +93,22 @@ func TestBasicSyncMode(t *testing.T) { // retrieve state delta gResp, err := followClient.GetLedgerStateDelta(round) a.NoError(err) - a.NotNil(gResp) + a.NotZero(gResp) + + if round == *txn.ConfirmedRound { + // Txleases should always be nil for JSON responses + require.Nil(t, gResp.Txleases) + + // Verify that the transaction is in the state delta + expectedTxids := map[transactions.Txid]ledgercore.IncludedTransactions{ + txn.Txn.ID(): { + LastValid: txn.Txn.Txn.LastValid, + Intra: 0, + }, + } + require.Equal(t, expectedTxids, gResp.Txids) + } + // set sync round next err = followClient.SetSyncRound(round + 1) a.NoError(err)