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

Improve list contracts by code query #497

Merged
merged 3 commits into from
Apr 27, 2021
Merged

Improve list contracts by code query #497

merged 3 commits into from
Apr 27, 2021

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Apr 23, 2021

Resolves #323

Breaking change:
List contracts by code returns now the addresses without other data from CodeInfo
This was a gas intensive query before. Especially with the legacy rest endpoint

@alpe alpe added the API Fixes to the external client api label Apr 23, 2021
@alpe alpe requested a review from ethanfrey April 23, 2021 13:15
@alpe alpe force-pushed the list_contract_323 branch from 0d732e5 to c377d71 Compare April 23, 2021 13:15
@codecov
Copy link

codecov bot commented Apr 23, 2021

Codecov Report

Merging #497 (8c7967e) into master (144a85a) will decrease coverage by 0.03%.
The diff coverage is 87.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
- Coverage   58.45%   58.41%   -0.04%     
==========================================
  Files          41       41              
  Lines        4441     4437       -4     
==========================================
- Hits         2596     2592       -4     
  Misses       1631     1631              
  Partials      214      214              
Impacted Files Coverage Δ
x/wasm/keeper/keeper.go 85.76% <80.00%> (-0.35%) ⬇️
x/wasm/keeper/legacy_querier.go 67.44% <100.00%> (-1.31%) ⬇️
x/wasm/keeper/querier.go 58.95% <100.00%> (-1.15%) ⬇️
x/wasm/types/keys.go 38.23% <100.00%> (ø)

@alpe alpe changed the title List contract 323 Improve list contracts by code query Apr 23, 2021
contrib/local/02-contracts.sh Show resolved Hide resolved
x/wasm/keeper/querier_test.go Outdated Show resolved Hide resolved
Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Good job with the code index, and looks like a good improvement. But I don't see any migration tests here.

Can you add one that explicitly shows a different ordering based on migration vs creation?

x/wasm/keeper/keeper.go Show resolved Hide resolved
x/wasm/keeper/keeper.go Show resolved Hide resolved
CodeID: 1,
Created: &AbsoluteTxPosition{2 + 1<<(8*7), 3 + 1<<(8*7)},
Copy link
Member

Choose a reason for hiding this comment

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

I would keep a different Created here just to prove it takes one and ignores the other.

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 have switched from ContractInfo to ContractCodeHistoryEntry element. Any instantiation/migration results in a new ContractCodeHistoryEntry so that it covers both.

x/wasm/keeper/keeper_test.go Show resolved Hide resolved
@alpe alpe requested review from webmaster128 and ethanfrey April 27, 2021 07:05
Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Looking good. Thanks for that test

x/wasm/keeper/keeper_test.go Show resolved Hide resolved
@ethanfrey ethanfrey merged commit c67cf14 into master Apr 27, 2021
@ethanfrey ethanfrey deleted the list_contract_323 branch April 27, 2021 09:38
@alpe alpe added api breaking and removed API Fixes to the external client api labels Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Correct order for migrated contracts
3 participants