Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Add GetTrustedHeader #390

Merged
merged 3 commits into from
Feb 2, 2021
Merged

Add GetTrustedHeader #390

merged 3 commits into from
Feb 2, 2021

Conversation

NagaTulasi
Copy link
Contributor

No description provided.

@NagaTulasi NagaTulasi marked this pull request as ready for review January 25, 2021 10:18
@anilcse anilcse changed the title funcion added Add GetTrusteddstHeader Jan 26, 2021
@colin-axner colin-axner self-requested a review February 1, 2021 11:42
colin-axner
colin-axner suggested changes Feb 1, 2021
View reviewed changes
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, I think we can simply the logic here by making the function a little more generic. The TODO can be removed as well along with adding an error check

relayer/headers.go Outdated
@@ -100,6 +100,16 @@ func (uh *SyncHeaders) GetTrustedHeaders(src, dst *Chain) (srcTh, dstTh *tmclien
return
}

func (uh *SyncHeaders) GetTrusteddsthHeader(src, dst *Chain) (dstTh *tmclient.Header, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe rename to GetTrustedHeader, but construct the call like:

sh.GetTrsutedHeader(dst, c)

note the swapping. This way we can reuse this function for source if needed

@NagaTulasi NagaTulasi changed the title Add GetTrusteddstHeader Add GetTrustedHeader Feb 2, 2021
colin-axner
colin-axner approved these changes Feb 2, 2021
View reviewed changes
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@colin-axner colin-axner merged commit cbedb3c into cosmos:master Feb 2, 2021
@colin-axner colin-axner mentioned this pull request Feb 2, 2021
Closed
@NagaTulasi NagaTulasi deleted the tulasi/add-dsth branch February 2, 2021 11:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@colin-axner colin-axner colin-axner approved these changes

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal

@fedekunze fedekunze Awaiting requested review from fedekunze

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NagaTulasi @colin-axner