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

API methods to get height closest to timestamp #344

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

noescape00
Copy link
Contributor

Added API methods to controller and client to request block height that is closed to given timestamp.

This is later will be needed to initialize NextMatureDepositHeight in CrossChainTransferStore so the store syncs not from the 1st block but from a block generated at the same time when genesis block of current chain was created.

It is not possible to do it now because CrossChainTransferStore requires refactoring in order to allow initialization that requires API call to another node.

related to #268

@bokobza
Copy link
Contributor

bokobza commented Dec 20, 2018

There is already an extension method that calculates the height of the block at a certain time: this.chain.GetHeightAtTime(date);

Also, can this not just be a constant value on the federation gateway mainchain?

@noescape00
Copy link
Contributor Author

Also, can this not just be a constant value on the federation gateway mainchain?

It can be. But it will make setup a bit more difficult because you will need to provide it in the arguments.

If we do it like that then we don't need this PR and API calls at all, just initialize height at that value and that's it.

But imho it's not user friendly. Right now it's not trivial to configure fed gateway already.

@bokobza
Copy link
Contributor

bokobza commented Dec 20, 2018

yeah but here it's such a lot of work to retrieve a value we already know.

Copy link
Contributor

@bokobza bokobza left a comment

Choose a reason for hiding this comment

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

blocking until after release.

@bokobza bokobza added the discussion Where a discussion is needed label Dec 20, 2018
@noescape00
Copy link
Contributor Author

noescape00 commented Dec 20, 2018

yeah but here it's such a lot of work to retrieve a value we already know.

Your decision. A lot of work for us or a bit of work during setting it up.

I don't mind either way.

If you prefer to have it in the setup config then close this PR and I'll do it that way.

ps
I'd keep changes in the rest client- there are methods for get requests. Rest of the PR is up to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Where a discussion is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants