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

Replace at-tip checks with utils functions from the latest sdk #212

Merged
merged 4 commits into from
Jan 12, 2021

Conversation

septerr
Copy link
Contributor

@septerr septerr commented Jan 12, 2021

Fixes # .

Motivation

Rosetta CLI checks to see if it is at tip at a few different places. These checks are not consistent.

For instance, before constructing transactions, the CLI checks if it is a tip based on (1) if the stored current block is within tip delay or (2) block returned by network/status is at tip (it is within tip delay or has sync status of true) and it is same as the stored current block.

Before broadcasting transactions, the CLI checks if it is at tip based only on if the stored current block is within tip delay.

These inconsistent checks were leading to the CLI getting stuck during transaction broadcast for chains that use sync status rather than tip delays.

Solution

Use utility functions from the sdk's utils package to do at-tip checks consistently.

Open questions

go.mod Outdated Show resolved Hide resolved
pkg/tester/data.go Outdated Show resolved Hide resolved
@patrick-ogrady
Copy link
Contributor

You'll also want to modify this: https://github.com/coinbase/rosetta-cli/blob/23e6daf53528a2695b05e6a7f584d4f5e31c1023/cmd/root.go#L272

@coveralls
Copy link

coveralls commented Jan 12, 2021

Pull Request Test Coverage Report for Build 6718

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 58.737%

Totals Coverage Status
Change from base Build 6692: 0.0%
Covered Lines: 521
Relevant Lines: 887

💛 - Coveralls

@septerr septerr requested review from patrick-ogrady and removed request for patrick-ogrady January 12, 2021 18:36
@patrick-ogrady patrick-ogrady merged commit f5f4d16 into master Jan 12, 2021
@patrick-ogrady patrick-ogrady deleted the sgupta/use-utils-checktip branch January 12, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants