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

Implement the endpoint GET /v2/block-headers #1638

Merged
merged 19 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
244 changes: 239 additions & 5 deletions api/converter_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,18 @@ func decodeDigest(str *string, field string, errorArr []string) (string, []strin
return "", errorArr
}

// decodeAddress returns the byte representation of the input string, or appends an error to errorArr
func decodeAddress(str *string, field string, errorArr []string) ([]byte, []string) {
// decodeAddress returns the sdk.Address representation of the input string, or appends an error to errorArr
func decodeAddress(str string, field string, errorArr []string) (sdk.Address, []string) {
Copy link
Contributor

@jannotti jannotti Nov 21, 2024

Choose a reason for hiding this comment

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

I see that decodeAddress already existed, so this PR is not to blame for the unusual way that multiple errors are accumulated, but I think there is a better way to handle multiple errors these days.

I would use sdk.DecodeAddress directly, and if it returns an error, append that into a []error instead of []string. Use fmt.Errorf with a %w to produce an error that wraps the error from sdk.DecodeAddress but also contains the str that would have been passed into decodeAddress. Then at the end of a function that may have accumulated errors, test its size just as you do, but then use errors.Join(errArr...) to combine them into a single error.

I'll leave it to others to decide if this is important enough to change. I believe it will make it easier to follow the error handling, but it isn't critical if this is a common pattern in indexer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, even nicer:
Join returns an error that wraps the given errors. Any nil error values are discarded. Join returns nil if every value in errs is nil.
therefore, you don't need to test the err that is returned from sdk.DecodeAddress, just append it to your error list. Then, at the bottom, don't check to see if the list has errors in it, just:

  err := errors.Join(errList...)
  if err != nil {
    return err
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addr, err := sdk.DecodeAddress(str)
if err != nil {
return sdk.ZeroAddress, append(errorArr, fmt.Sprintf("%s '%s': %v", errUnableToParseAddress, field, err))
}
// Pass through
return addr, errorArr
}

// decodeAddressToBytes returns the byte representation of the input string, or appends an error to errorArr
func decodeAddressToBytes(str *string, field string, errorArr []string) ([]byte, []string) {
if str != nil {
addr, err := sdk.DecodeAddress(*str)
if err != nil {
Expand Down Expand Up @@ -298,6 +308,94 @@ func txnRowToTransaction(row idb.TxnRow) (generated.Transaction, error) {
return txn, nil
}

func rowToBlock(blockHeader *sdk.BlockHeader) generated.Block {
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't written much indexer code, but this name seems a bit underspecified. Above, we see txnRowToTransaction, so this should probably be hdrRowToBlock. I'm not totally confident about this recommendation because I would have expected this to take some sort of idb type, rather than an sdk type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fair, the name hdrRowToBlock will generate less confusion when reading the code.

Addressed in eaefec7 and 8924c97


rewards := generated.BlockRewards{
FeeSink: blockHeader.FeeSink.String(),
RewardsCalculationRound: uint64(blockHeader.RewardsRecalculationRound),
RewardsLevel: blockHeader.RewardsLevel,
RewardsPool: blockHeader.RewardsPool.String(),
RewardsRate: blockHeader.RewardsRate,
RewardsResidue: blockHeader.RewardsResidue,
}

upgradeState := generated.BlockUpgradeState{
CurrentProtocol: string(blockHeader.CurrentProtocol),
NextProtocol: strPtr(string(blockHeader.NextProtocol)),
NextProtocolApprovals: uint64Ptr(blockHeader.NextProtocolApprovals),
NextProtocolSwitchOn: uint64Ptr(uint64(blockHeader.NextProtocolSwitchOn)),
NextProtocolVoteBefore: uint64Ptr(uint64(blockHeader.NextProtocolVoteBefore)),
}

upgradeVote := generated.BlockUpgradeVote{
UpgradeApprove: boolPtr(blockHeader.UpgradeApprove),
UpgradeDelay: uint64Ptr(uint64(blockHeader.UpgradeDelay)),
UpgradePropose: strPtr(string(blockHeader.UpgradePropose)),
}

var partUpdates *generated.ParticipationUpdates = &generated.ParticipationUpdates{}
if len(blockHeader.ExpiredParticipationAccounts) > 0 {
addrs := make([]string, len(blockHeader.ExpiredParticipationAccounts))
for i := 0; i < len(addrs); i++ {
addrs[i] = blockHeader.ExpiredParticipationAccounts[i].String()
}
partUpdates.ExpiredParticipationAccounts = strArrayPtr(addrs)
}
if len(blockHeader.AbsentParticipationAccounts) > 0 {
addrs := make([]string, len(blockHeader.AbsentParticipationAccounts))
for i := 0; i < len(addrs); i++ {
addrs[i] = blockHeader.AbsentParticipationAccounts[i].String()
}
partUpdates.AbsentParticipationAccounts = strArrayPtr(addrs)
}
if *partUpdates == (generated.ParticipationUpdates{}) {
partUpdates = nil
}

// order these so they're deterministic
orderedTrackingTypes := make([]sdk.StateProofType, len(blockHeader.StateProofTracking))
trackingArray := make([]generated.StateProofTracking, len(blockHeader.StateProofTracking))
elems := 0
for key := range blockHeader.StateProofTracking {
orderedTrackingTypes[elems] = key
elems++
}
sort.Slice(orderedTrackingTypes, func(i, j int) bool { return orderedTrackingTypes[i] < orderedTrackingTypes[j] })
Copy link
Contributor

Choose a reason for hiding this comment

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

slices.Sort is a more convenient, and usually faster, way to sort.

Copy link
Contributor Author

@agodnic agodnic Nov 21, 2024

Choose a reason for hiding this comment

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

Addressed in 82e864b.

There are more instances of the sort.Slice function being used, but I don't want to create scope creep. Can totally do this change (potentially in a separate PR) if you guys want.

for i := 0; i < len(orderedTrackingTypes); i++ {
stpfTracking := blockHeader.StateProofTracking[orderedTrackingTypes[i]]
thing1 := generated.StateProofTracking{
NextRound: uint64Ptr(uint64(stpfTracking.StateProofNextRound)),
Type: uint64Ptr(uint64(orderedTrackingTypes[i])),
VotersCommitment: byteSliceOmitZeroPtr(stpfTracking.StateProofVotersCommitment),
OnlineTotalWeight: uint64Ptr(uint64(stpfTracking.StateProofOnlineTotalWeight)),
}
trackingArray[orderedTrackingTypes[i]] = thing1
}

ret := generated.Block{
Bonus: uint64PtrOrNil(uint64(blockHeader.Bonus)),
FeesCollected: uint64PtrOrNil(uint64(blockHeader.FeesCollected)),
GenesisHash: blockHeader.GenesisHash[:],
GenesisId: blockHeader.GenesisID,
ParticipationUpdates: partUpdates,
PreviousBlockHash: blockHeader.Branch[:],
Proposer: addrPtr(blockHeader.Proposer),
ProposerPayout: uint64PtrOrNil(uint64(blockHeader.ProposerPayout)),
Rewards: &rewards,
Round: uint64(blockHeader.Round),
Seed: blockHeader.Seed[:],
StateProofTracking: &trackingArray,
Timestamp: uint64(blockHeader.TimeStamp),
Transactions: nil,
TransactionsRoot: blockHeader.TxnCommitments.NativeSha512_256Commitment[:],
TransactionsRootSha256: blockHeader.TxnCommitments.Sha256Commitment[:],
TxnCounter: uint64Ptr(blockHeader.TxnCounter),
UpgradeState: &upgradeState,
UpgradeVote: &upgradeVote,
}
return ret
}

func signedTxnWithAdToTransaction(stxn *sdk.SignedTxnWithAD, extra rowData) (generated.Transaction, error) {
var payment *generated.TransactionPayment
var keyreg *generated.TransactionKeyreg
Expand Down Expand Up @@ -640,7 +738,7 @@ func edIndexToAddress(index uint64, txn sdk.Transaction, shared []sdk.Address) (
}

func (si *ServerImplementation) assetParamsToAssetQuery(params generated.SearchForAssetsParams) (idb.AssetsQuery, error) {
creator, errorArr := decodeAddress(params.Creator, "creator", make([]string, 0))
creator, errorArr := decodeAddressToBytes(params.Creator, "creator", make([]string, 0))
if len(errorArr) != 0 {
return idb.AssetsQuery{}, errors.New(errUnableToParseAddress)
Copy link
Contributor

Choose a reason for hiding this comment

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

As an example of what I described earlier, I think this reads much better, without needing to understand the unusual error handling model of decodeAddress.

Suggested change
creator, errorArr := decodeAddressToBytes(params.Creator, "creator", make([]string, 0))
if len(errorArr) != 0 {
return idb.AssetsQuery{}, errors.New(errUnableToParseAddress)
creator, err := sdk.DecodeAddress(params.Creator)
if err {
return idb.AssetsQuery{}, fmt.Errorf("Unable to parse creator address: %v", err)

It's worth noting that the existing code supplies the word "creator" to decodeAddressToBytes in order to get a nicer error message, and then immediately throws that nice error message away in favor of a generic message.

You can also just use creator[:] in the code below, rather than have separate functions to decode to an sdk.Address, or to []byte.

Copy link
Contributor Author

@agodnic agodnic Nov 22, 2024

Choose a reason for hiding this comment

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

This change makes a lot of sense:

  • removes unnecessary code (there are two functions that pretty much do the same thing as sdk.DecodeAddress)
  • the logic in those functions is somewhat confusing to the reader
  • handling errors as []error is preferrable over []string

Commit 7f26e0f adds more context information to the errors being returned. If this is considered a breaking change, I can make it return the same exact old error messages.

I'm not a fan of returning multiple errors, I think it is not worth the extra complexity compared to a fail-fast approach (returning upon the first error encountered). In fact this could introduce bugs, since the control flow keeps advancing with variables that may be in an unknown state (I'd rather stop the control flow immediately upon finding something that doesn't check out).
But there may be a reason why multiple errors are being used that I'm not aware of.

There are other instances in the codebase that are handling multiple errors as []string instead of []error. I didn't change all of these to avoid scope creep, but could do so in a separate PR.

}
Expand Down Expand Up @@ -669,7 +767,7 @@ func (si *ServerImplementation) assetParamsToAssetQuery(params generated.SearchF
}

func (si *ServerImplementation) appParamsToApplicationQuery(params generated.SearchForApplicationsParams) (idb.ApplicationQuery, error) {
addr, errorArr := decodeAddress(params.Creator, "creator", make([]string, 0))
addr, errorArr := decodeAddressToBytes(params.Creator, "creator", make([]string, 0))
if len(errorArr) != 0 {
return idb.ApplicationQuery{}, errors.New(errUnableToParseAddress)
}
Expand Down Expand Up @@ -708,7 +806,7 @@ func (si *ServerImplementation) transactionParamsToTransactionFilter(params gene
filter.NextToken = strOrDefault(params.Next)

// Address
filter.Address, errorArr = decodeAddress(params.Address, "address", errorArr)
filter.Address, errorArr = decodeAddressToBytes(params.Address, "address", errorArr)
filter.Txid, errorArr = decodeDigest(params.Txid, "txid", errorArr)

// Byte array
Expand Down Expand Up @@ -749,6 +847,142 @@ func (si *ServerImplementation) transactionParamsToTransactionFilter(params gene
return
}

func (si *ServerImplementation) blockParamsToBlockFilter(params generated.SearchForBlocksParams) (filter idb.BlockFilter, err error) {
var errorArr []string

// Integer
filter.Limit = min(uintOrDefaultValue(params.Limit, si.opts.DefaultBlocksLimit), si.opts.MaxBlocksLimit)
// If min/max are mixed up
//
// This check is performed here instead of in validateBlockFilter because
// when converting params into a filter, the next token is merged with params.MinRound.
if params.MinRound != nil && params.MaxRound != nil && *params.MinRound > *params.MaxRound {
errorArr = append(errorArr, errInvalidRoundMinMax)
}
filter.MaxRound = params.MaxRound
filter.MinRound = params.MinRound

// String
if params.Next != nil {
n, err := idb.DecodeBlockRowNext(*params.Next)
if err != nil {
errorArr = append(errorArr, fmt.Sprintf("%s: %v", errUnableToParseNext, err))
}
// Set the MinRound
if filter.MinRound == nil {
filter.MinRound = uint64Ptr(n + 1)
} else {
filter.MinRound = uint64Ptr(max(*filter.MinRound, n+1))
}
}

// Time
if params.AfterTime != nil {
filter.AfterTime = *params.AfterTime
}
if params.BeforeTime != nil {
filter.BeforeTime = *params.BeforeTime
}

// Address list
{
// Make sure at most one of the participation parameters is set
numParticipationFilters := 0
if params.Proposer != nil {
numParticipationFilters++
}
if params.Expired != nil {
numParticipationFilters++
}
if params.Absent != nil {
numParticipationFilters++
}
if params.Updates != nil {
numParticipationFilters++
}
if params.Participation != nil {
numParticipationFilters++
}
if numParticipationFilters > 1 {
errorArr = append(errorArr, "only one of `proposer`, `expired`, `absent`, `updates`, or `participation` can be specified")
}

// Validate the number of items in the participation account lists
if params.Proposer != nil && uint64(len(*params.Proposer)) > si.opts.MaxAccountListSize {
errorArr = append(errorArr, fmt.Sprintf("proposer list too long, max size is %d", si.opts.MaxAccountListSize))
}
if params.Expired != nil && uint64(len(*params.Expired)) > si.opts.MaxAccountListSize {
errorArr = append(errorArr, fmt.Sprintf("expired list too long, max size is %d", si.opts.MaxAccountListSize))
}
if params.Absent != nil && uint64(len(*params.Absent)) > si.opts.MaxAccountListSize {
errorArr = append(errorArr, fmt.Sprintf("absent list too long, max size is %d", si.opts.MaxAccountListSize))
}
if params.Updates != nil && uint64(len(*params.Updates)) > si.opts.MaxAccountListSize {
errorArr = append(errorArr, fmt.Sprintf("updates list too long, max size is %d", si.opts.MaxAccountListSize))
}
if params.Participation != nil && uint64(len(*params.Participation)) > si.opts.MaxAccountListSize {
errorArr = append(errorArr, fmt.Sprintf("participation list too long, max size is %d", si.opts.MaxAccountListSize))
}

filter.Proposers = make(map[sdk.Address]struct{}, 0)
if params.Proposer != nil {
for _, s := range *params.Proposer {
var addr sdk.Address
addr, errorArr = decodeAddress(s, "proposer", errorArr)
filter.Proposers[addr] = struct{}{}
}
}

filter.ExpiredParticipationAccounts = make(map[sdk.Address]struct{}, 0)
if params.Expired != nil {
for _, s := range *params.Expired {
var addr sdk.Address
addr, errorArr = decodeAddress(s, "expired", errorArr)
filter.ExpiredParticipationAccounts[addr] = struct{}{}
}
}

filter.AbsentParticipationAccounts = make(map[sdk.Address]struct{}, 0)
if params.Absent != nil {
for _, s := range *params.Absent {
var addr sdk.Address
addr, errorArr = decodeAddress(s, "absent", errorArr)
filter.AbsentParticipationAccounts[addr] = struct{}{}
}
}

// Updates = absent || expired
if params.Updates != nil {
for _, s := range *params.Updates {
var addr sdk.Address
addr, errorArr = decodeAddress(s, "updates", errorArr)
filter.AbsentParticipationAccounts[addr] = struct{}{}
filter.ExpiredParticipationAccounts[addr] = struct{}{}
}
}

// Participation = proposer || absent || expired
if params.Participation != nil {
for _, s := range *params.Participation {
var addr sdk.Address
addr, errorArr = decodeAddress(s, "participation", errorArr)
filter.Proposers[addr] = struct{}{}
filter.AbsentParticipationAccounts[addr] = struct{}{}
filter.ExpiredParticipationAccounts[addr] = struct{}{}
}
}
}

// If there were any errorArr while setting up the BlockFilter, return now.
if len(errorArr) > 0 {
err = errors.New("invalid input: " + strings.Join(errorArr, ", "))

// clear out the intermediates.
filter = idb.BlockFilter{}
}
return
}

func (si *ServerImplementation) maxAccountsErrorToAccountsErrorResponse(maxErr idb.MaxAPIResourcesPerAccountError) generated.ErrorResponse {
addr := maxErr.Address.String()
max := uint64(si.opts.MaxAPIResourcesPerAccount)
Expand Down
2 changes: 2 additions & 0 deletions api/error_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
const (
errInvalidRoundAndMinMax = "cannot specify round and min-round/max-round"
errInvalidRoundMinMax = "min-round must be less than max-round"
errInvalidTimeMinMax = "after-time must be less than before-time"
errUnableToParseAddress = "unable to parse address"
errInvalidCreatorAddress = "found an invalid creator address"
errUnableToParseBase64 = "unable to parse base64 data"
Expand Down Expand Up @@ -38,6 +39,7 @@ const (
ErrFailedLookingUpBoxes = "failed while looking up application boxes"
errRewindingAccountNotSupported = "rewinding account is no longer supported, please remove the `round=` query parameter and try again"
errLookingUpBlockForRound = "error while looking up block for round"
errBlockHeaderSearch = "error while searching for block headers"
errTransactionSearch = "error while searching for transaction"
errZeroAddressCloseRemainderToRole = "searching transactions by zero address with close address role is not supported"
errZeroAddressAssetSenderRole = "searching transactions by zero address with asset sender role is not supported"
Expand Down
Loading