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

feat: improve block height specification for query #101

Merged
merged 9 commits into from
Mar 10, 2023

Conversation

gyuguen
Copy link
Contributor

@gyuguen gyuguen commented Mar 7, 2023

close #100

Implementation

  • Removed on QueryMiddelware
  • Fix to get last block information every 3 seconds and cache the height of this block
  • Upgrade COSMOS-SDK version to mainnet (V2.0.6) as unnecessary logs continue to be outputted

@gyuguen gyuguen added this to the v0.0.1-alpha.2 milestone Mar 7, 2023
@gyuguen gyuguen self-assigned this Mar 7, 2023
@@ -59,12 +57,6 @@ func (s *PanaceaSubscriber) subscribe(event Event) error {
for tx := range txs {
ctx := context.Background()

height, err := strconv.ParseInt(tx.Events["tx.height"][0], 10, 64)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer take the block height received as an event and use it .
We always use the cached block height.

mutex *sync.Mutex
cdc *codec.ProtoCodec
aminoCdc *codec.AminoCodec
cachedLastBlockHeight int64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cached block height is managed here.

}, nil
}

go queryClient.startSchedulingLastBlockCaching()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made some modifications to the existing refresh().

}
return q.GetLastBlockHeight(ctx)
func (q *verifiedQueryClient) getQueryBlockHeight() int64 {
return q.cachedLastBlockHeight - 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always do -1 in the last block, because the appHash is always in the next block. This is the same as the previous spec.

Copy link
Contributor

@audtlr24 audtlr24 left a comment

Choose a reason for hiding this comment

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

Thanks, left some comments on some deprecated functions.

panacea/query_client.go Show resolved Hide resolved
panacea/query_client.go Show resolved Hide resolved
panacea/query_client.go Show resolved Hide resolved
panacea/query_client.go Show resolved Hide resolved
panacea/query_client.go Show resolved Hide resolved
@gyuguen gyuguen changed the base branch from main to tags/v0.0.1-alpha.1 March 9, 2023 05:07
@gyuguen gyuguen changed the base branch from tags/v0.0.1-alpha.1 to tags/v0.0.1-alpha March 9, 2023 05:09
@gyuguen gyuguen changed the base branch from tags/v0.0.1-alpha to release/v0.0.1 March 9, 2023 05:10
Copy link
Contributor

@0xHansLee 0xHansLee left a comment

Choose a reason for hiding this comment

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

lgtm

@gyuguen
Copy link
Contributor Author

gyuguen commented Mar 9, 2023

@audtlr24 @H4NLee @youngjoon-lee
I re-requested the review because I changed the logic for caching the last block information.

Copy link
Contributor

@youngjoon-lee youngjoon-lee left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@0xHansLee 0xHansLee left a comment

Choose a reason for hiding this comment

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

Good

Copy link
Contributor

@audtlr24 audtlr24 left a comment

Choose a reason for hiding this comment

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

LGTM!

go.mod Show resolved Hide resolved
@gyuguen gyuguen merged commit 4e88f90 into release/v0.0.1 Mar 10, 2023
@gyuguen gyuguen deleted the ft/100/improve_query_block_height branch March 10, 2023 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved block height specification for query
5 participants