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

General improvements and fixes found during testing #72

Merged

Conversation

devbugging
Copy link
Contributor

@devbugging devbugging commented Feb 21, 2024

This PR includes multiple improvements for issues found during testing, some are:

  • Improve general API error handling to return null instead of not found errors as the clients expect such behaviour,
  • track nonce with block height so if a nonce is updated for same block height is not incremented
  • Improved logging and error descriptions

@devbugging devbugging changed the base branch from main to gregor/index/track-flow-heights February 22, 2024 21:54
@devbugging devbugging marked this pull request as ready for review February 22, 2024 21:54
Gregor Gololicic added 2 commits February 22, 2024 23:16
}

highestIndex := len(block.TransactionHashes) - 1
if index > hexutil.Uint(highestIndex) {
return nil
return nil, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we should return some kind of error here, instead of both values being nil, nil.

}

// todo workaround until new version of flow-go is released
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice one 👍 I think the evm.TransactionExecuted event payload is going to contain BlockHash, so we can avoid the extra DB call.

Copy link
Collaborator

@m-Peter m-Peter left a comment

Choose a reason for hiding this comment

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

Very nice 👏
Left only a few comments for discussion.

@devbugging devbugging merged commit f358376 into gregor/index/track-flow-heights Feb 27, 2024
1 of 5 checks passed
@devbugging devbugging mentioned this pull request Feb 27, 2024
Merged
@m-Peter m-Peter deleted the gregor/index/previewnet-testing-fixes branch March 8, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants