fix address range functions by pronvis · Pull Request #316 · CosmWasm/cw-plus

pronvis · 2021-06-30T15:28:43Z

No description provided.

CLAassistant · 2021-06-30T15:28:47Z

All committers have signed the CLA.

orkunkl

LGTM. tests for extra coverage would be great if you will like coding more.
Let's wait for core maintainers approval for merging.

pronvis · 2021-06-30T16:09:11Z

Do not merge please. I will add some tests

pronvis · 2021-07-01T09:15:09Z

Tests added. (hope you are OK with that fact that I add dev-dependency to wrote tests

pronvis · 2021-07-01T09:18:37Z

cargo fmt does not change anything. Don't know why lint fails

maurolacy · 2021-07-02T06:51:55Z

cargo fmt does not change anything. Don't know why lint fails

Likely, some version / rules updates.

Check https://app.circleci.com/pipelines/github/CosmWasm/cosmwasm-plus/1252/workflows/4623eeff-43b1-4e5b-aba5-970584ca9f55/jobs/23104/parallel-runs/0/steps/0-107

pronvis · 2021-07-02T08:15:22Z

@maurolacy there is no way to log in in circle without providing This application will be able to read and write all public and private repository data. which I do not like.
So, can't open your url.
I did rustup update, state:

stable-x86_64-apple-darwin updated - rustc 1.53.0 (53cb7b09b 2021-06-17) (from rustc 1.52.1 (9bc8c42bb 2021-05-09))
nightly-x86_64-apple-darwin updated - rustc 1.55.0-nightly (7100b311d 2021-07-01) (from rustc 1.54.0-nightly (1c6868aa2 2021-05-27))

Still no changes from cargo fmt, so have no idea how to fix.

maurolacy · 2021-07-02T08:37:26Z

@maurolacy there is no way to log in in circle without providing This application will be able to read and write all public and private repository data. which I do not like.
So, can't open your url.
I did rustup update, state:
stable-x86_64-apple-darwin updated - rustc 1.53.0 (53cb7b09b 2021-06-17) (from rustc 1.52.1 (9bc8c42bb 2021-05-09))
nightly-x86_64-apple-darwin updated - rustc 1.55.0-nightly (7100b311d 2021-07-01) (from rustc 1.54.0-nightly (1c6868aa2 2021-05-27))
Still no changes from cargo fmt, so have no idea how to fix.

Don't worry, we'll fix it later.

maurolacy · 2021-07-02T09:17:04Z

Still no changes from cargo fmt, so have no idea how to fix.

You can always try and reproduce the CI setup / tests locally. Take a look a the cicleci config file for reference.

pronvis · 2021-07-02T10:56:33Z

cargo clippy --all-targets -- -D warnings helped

webmaster128

Just some code style remarks. Mauro and Ethan know the logic better than I do.

webmaster128 · 2021-07-06T22:06:47Z

packages/cw0/Cargo.toml

 thiserror = { version = "1.0.21" }
+
+[dev-dependencies]
+cw-storage-plus = { version = "0.6.1", features = ["iterator"] }


Can you use a local dependency, e.g.

Suggested change

cw-storage-plus = { version = "0.6.1", features = ["iterator"] }

cw-storage-plus = { path = "../../packages/storage-plus", version = "0.6.2", features = ["iterator"] }

webmaster128 · 2021-07-06T22:08:27Z

packages/cw0/src/pagination.rs

-    }
+pub fn calc_range_start(start_after: Option<Addr>) -> Option<Vec<u8>> {
+    start_after.map(|addr| {
+        let mut v: Vec<u8> = addr.as_ref().into();


There is now .as_bytes() which is a bit more explicit and does the same

Suggested change

let mut v: Vec<u8> = addr.as_ref().into();

let mut v: Vec<u8> = addr.as_bytes().into();

webmaster128 · 2021-07-06T22:08:42Z

packages/cw0/src/pagination.rs

-        None => Ok(None),
-    }
+pub fn calc_range_end(end_before: Option<Addr>) -> Option<Vec<u8>> {
+    end_before.map(|addr| addr.as_ref().into())


Suggested change

end_before.map(|addr| addr.as_ref().into())

end_before.map(|addr| addr.as_bytes().into())

maurolacy · 2021-07-07T05:33:55Z

packages/cw0/src/pagination.rs

-        }
-        None => Ok(None),
-    }
+pub fn calc_range_start(start_after: Option<Addr>) -> Option<Vec<u8>> {


Can only do proper / full reviews beginning next week.

Perhaps the original functions must be kept, as they can be useful for old-style ranges / storage. These new ones can also be suffixed by _addr for clarity, but given that this is the new standard, I don't think that's really necessary.

I see now that the original argument is Addr already so, it's OK to remove / rename these functions.

maurolacy

lgtm.

pronvis · 2021-07-07T09:28:51Z

  --> packages/cw0/src/pagination.rs:16:35
   |
16 |         let mut v: Vec<u8> = addr.as_bytes().into();
   |                                   ^^^^^^^^ method not found in `Addr`

error[E0599]: no method named `as_bytes` found for struct `Addr` in the current scope
  --> packages/cw0/src/pagination.rs:24:32
   |
24 |     end_before.map(|addr| addr.as_bytes().into())
   |                                ^^^^^^^^ method not found in `Addr`

Fix dependency section.

webmaster128 · 2021-07-07T12:35:49Z

^^^^^^^ method not found in Addr

The version of cosmwasm-std is not up to date anymore. Your branch has 0.14. Could you update this PR to the latest main state?

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

pronvis · 2021-07-07T12:49:39Z

^^^^^^^ method not found in Addr

The version of cosmwasm-std is not up to date anymore. Your branch has 0.14. Could you update this PR to the latest main state?

Done, rebase it to main

orkunkl · 2021-07-12T10:18:20Z

Merging

orkunkl approved these changes Jun 30, 2021

View reviewed changes

orkunkl self-requested a review June 30, 2021 15:31

orkunkl requested review from ethanfrey and maurolacy July 5, 2021 12:02

webmaster128 reviewed Jul 6, 2021

View reviewed changes

maurolacy reviewed Jul 7, 2021

View reviewed changes

maurolacy approved these changes Jul 7, 2021

View reviewed changes

pronvis added 5 commits July 7, 2021 15:45

fix address range functions

d35ea3f

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

add tests to address range functions

64faf69

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

fix linter warnings on added tests

68b8d5b

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

update dependency section to link to local path

05b2f82

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

rebase to main branch

0509cc7

Signed-off-by: Stanislav Pirx <stanislav.pirx@gmail.com>

pronvis force-pushed the fix_addr_range_functions branch from b833b61 to 0509cc7 Compare July 7, 2021 12:48

orkunkl merged commit c6f8663 into CosmWasm:main Jul 12, 2021

	cw-storage-plus = { version = "0.6.1", features = ["iterator"] }
	cw-storage-plus = { path = "../../packages/storage-plus", version = "0.6.2", features = ["iterator"] }

	let mut v: Vec<u8> = addr.as_ref().into();
	let mut v: Vec<u8> = addr.as_bytes().into();

	end_before.map(\|addr\| addr.as_ref().into())
	end_before.map(\|addr\| addr.as_bytes().into())

Conversation

pronvis commented Jun 30, 2021

Uh oh!

CLAassistant commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orkunkl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pronvis commented Jun 30, 2021

Uh oh!

pronvis commented Jul 1, 2021

Uh oh!

pronvis commented Jul 1, 2021

Uh oh!

maurolacy commented Jul 2, 2021

Uh oh!

pronvis commented Jul 2, 2021

Uh oh!

maurolacy commented Jul 2, 2021

Uh oh!

maurolacy commented Jul 2, 2021

Uh oh!

pronvis commented Jul 2, 2021

Uh oh!

webmaster128 left a comment

Choose a reason for hiding this comment

Uh oh!

webmaster128 Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

webmaster128 Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

webmaster128 Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

maurolacy Jul 7, 2021

Choose a reason for hiding this comment

Uh oh!

maurolacy Jul 7, 2021

Choose a reason for hiding this comment

Uh oh!

maurolacy left a comment

Choose a reason for hiding this comment

Uh oh!

pronvis commented Jul 7, 2021

Uh oh!

webmaster128 commented Jul 7, 2021

Uh oh!

pronvis commented Jul 7, 2021

Uh oh!

orkunkl commented Jul 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Jun 30, 2021 •

edited

Loading

orkunkl left a comment •

edited

Loading