Grant-Milestone-Delivery icon indicating copy to clipboard operation
Grant-Milestone-Delivery copied to clipboard

Subalfred milestone 1

Open aurexav opened this issue 2 years ago • 7 comments

Milestone Delivery Checklist

Link to the application pull request: https://github.com/w3f/Grants-Program/pull/1158.


~~Is the invoice form necessary?~~

aurexav avatar Oct 31 '22 18:10 aurexav

Hey @AurevoirXavier, thank you for the delivery. We'll look into it as soon as possible.

The invoice form helps you make sure that your invoice contains all the necessary information and we use it to coordinate the payment with the finance team. We would appreciate it if you could fill it out.

alxs avatar Oct 31 '22 18:10 alxs

Hey @AurevoirXavier, thank you for the delivery. We'll look into it as soon as possible.

Thanks for the reply.

I've filled up the invoice.

Usually, how long will this process take?

aurexav avatar Nov 03 '22 09:11 aurexav

Thank you. The length of the review process depends on how quickly an evaluator has the capacity to pick up your delivery, and the amount of changes required to it. Someone will most certainly get back to you in the next two weeks. Once the delivery has been accepted, payment happens within 14 days.

alxs avatar Nov 03 '22 10:11 alxs

Thanks for the reply/review!

  • The tests currently fail on my machine. I ran cargo test and got:
    running 1 test
    test src/lib.rs - impl_apis (line 13) ... FAILED
    
    failures:
    
    ---- src/lib.rs - impl_apis (line 13) stdout ----
    error: cannot find macro `impl_apis` in this scope
     --> src/lib.rs:14:1
      |
    3 | impl_apis! {
      | ^^^^^^^^^
      |
      = note: consider importing this macro:
              subrpcer::impl_apis
    
    error: aborting due to previous error
    
    Couldn't compile the test.
    
    failures:
        src/lib.rs - impl_apis (line 13)
    
    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s
    
    error: test failed, to rerun pass '--doc'
    

Fixed by https://github.com/hack-ink/subalfred/pull/275. Because I'm using cargo-nextest. It didn't check these.

  • Also, it seems like I can't install the deps, when running cargo install the following error is thrown:
    subalfred % cargo install               
    error: found a virtual manifest at `/Users/xxx/repos/subalfred/Cargo.toml` instead of a package manifest
    

Because this crate is still in RC state. And this is a restriction from crates.io. We need to specify the version:

cargo install subalfred --version 0.9.0-rc16
  • Finally, I'd have expected the deliverable 0d. to contain the Docker infrastructure, but currently it's just the same comment as on 0c. - A copy-paste error?

Actually, the docker image is a Linux Rust ENV.

I realize that it is much easier for you to test this with a simple cargo test command. So, I decide to use cargo instead of docker.

aurexav avatar Nov 08 '22 10:11 aurexav

@AurevoirXavier thanks for the quick response and update.

Fixed by https://github.com/hack-ink/subalfred/pull/275.

You basically ignored the test, instead of fixing it. Could you elaborate on the reason for that? If it's an example test that was somehow scaffolded and is not being used anymore, I think it'd be better to remove it. If it's a test you wrote manually and it still makes sense, then it's probably better to fix it.

I realize that it is much easier for you to test this with a simple cargo test command. So, I decide to use cargo instead of docker.

Makes sense. In that case, could you amend your contract and remove the Docker deliverable? And maybe also include a comment as to why you'd remove it. Small amendments like this one are usually quick to be approved by the grants committee.

I'm going to do another iteration on your delivery in the meanwhile.

takahser avatar Nov 09 '22 09:11 takahser

Hey! Thanks!

@AurevoirXavier thanks for the quick response and update.

Fixed by hack-ink/subalfred#275.

You basically ignored the test, instead of fixing it. Could you elaborate on the reason for that? If it's an example test that was somehow scaffolded and is not being used anymore, I think it'd be better to remove it. If it's a test you wrote manually and it still makes sense, then it's probably better to fix it.

This is an example to show how to use it which was not supposed to be a test. But cargo will test all the code block things. And the no_run flag doesn't work. Finally, I decide to use the ignore flag.

Please check:

  • https://github.com/paritytech/substrate/blob/cbb48e5e503300817b6837d6597659ed35212e99/frame/benchmarking/src/lib.rs#L74-L86
  • https://paritytech.github.io/substrate/master/frame_benchmarking/macro.benchmarks.html

I realize that it is much easier for you to test this with a simple cargo test command. So, I decide to use cargo instead of docker.

Makes sense. In that case, could you amend your contract and remove the Docker deliverable? And maybe also include a comment as to why you'd remove it. Small amendments like this one are usually quick to be approved by the grants committee.

I'm going to do another iteration on your delivery in the meanwhile.

Okay. I'll open a PR to update the application detail.

aurexav avatar Nov 09 '22 09:11 aurexav

@AurevoirXavier

This is an example to show how to use it which was not supposed to be a test.

Thanks for educating me, I didn't know about that :)

I'll open a PR to update the application detail.

Ok great 👍

takahser avatar Nov 09 '22 12:11 takahser

@AurevoirXavier thanks for your patience. I took a closer look and I've published my evaluation. However, I have two questions:

  1. I wasn't sure how to test deliverable 7 (--at flag). I didn't find any documentation on the Subalfred book. Could you provide some guidance here?
  2. Are you aware of the clippy issues? Is there a reason you didn't fix them (I only included a few of them, there are actually many more)?
% cargo clippy
    Checking log v0.4.17
    Checking socket2 v0.4.7
    Checking num_cpus v1.13.1
    Checking tracing v0.1.37
    Checking generic-array v0.14.6
    Checking futures-channel v0.3.25
    Checking getrandom v0.2.8
    Checking core-foundation v0.9.3
    Checking security-framework-sys v2.6.1
    Checking ring v0.16.20
error[E0514]: found crate `cfg_if` compiled by an incompatible version of rustc
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:187:1
    |
187 | extern crate cfg_if;
    | ^^^^^^^^^^^^^^^^^^^^
    |
    = note: the following crate versions were found:
            crate `cfg_if` compiled by rustc 1.64.0-nightly (93ffde6f0 2022-07-23): /Users/xxx/repos/subalfred/target/debug/deps/libcfg_if-3d8d0b265f4a3336.rmeta
    = help: please recompile that crate using this compiler (rustc 1.66.0-nightly (a37499ae6 2022-09-18)) (consider running `cargo clean` first)

error[E0514]: found crate `cfg_if` compiled by an incompatible version of rustc
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/log-0.4.17/src/lib.rs:331:1
    |
331 | extern crate cfg_if;
    | ^^^^^^^^^^^^^^^^^^^^
    |
    = note: the following crate versions were found:
            crate `cfg_if` compiled by rustc 1.64.0-nightly (93ffde6f0 2022-07-23): /Users/xxx/repos/subalfred/target/debug/deps/libcfg_if-3d8d0b265f4a3336.rmeta
    = help: please recompile that crate using this compiler (rustc 1.66.0-nightly (a37499ae6 2022-09-18)) (consider running `cargo clean` first)

error[E0514]: found crate `futures_sink` compiled by an incompatible version of rustc
 --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-channel-0.3.25/src/mpsc/sink_impl.rs:3:5
  |
3 | use futures_sink::Sink;
  |     ^^^^^^^^^^^^
  |
  = note: the following crate versions were found:
          crate `futures_sink` compiled by rustc 1.64.0-nightly (93ffde6f0 2022-07-23): /Users/xxx/repos/subalfred/target/debug/deps/libfutures_sink-7d03f35f3fad85c5.rmeta
  = help: please recompile that crate using this compiler (rustc 1.66.0-nightly (a37499ae6 2022-09-18)) (consider running `cargo clean` first)

error: cannot find macro `cfg_if` in this scope
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/error.rs:102:1
    |
102 | cfg_if! {
    | ^^^^^^
    |
note: `cfg_if` is imported here, but it is an unresolved item, not a macro
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:187:1
    |
187 | extern crate cfg_if;
    | ^^^^^^^^^^^^^^^^^^^^

error: cannot find macro `cfg_if` in this scope
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:203:1
    |
203 | cfg_if! {
    | ^^^^^^
    |
note: `cfg_if` is imported here, but it is an unresolved item, not a macro
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:187:1
    |
187 | extern crate cfg_if;
    | ^^^^^^^^^^^^^^^^^^^^

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:290:5
    |
290 |     imp::getrandom_inner(dest)
    |     ^^^ use of undeclared crate or module `imp`

error[E0425]: cannot find function `os_err` in this scope
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/error.rs:128:32
    |
128 |             if let Some(err) = os_err(errno, &mut buf) {
    |                                ^^^^^^ not found in this scope

error[E0425]: cannot find function `os_err` in this scope
   --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/error.rs:145:19
    |
145 |             match os_err(errno, &mut buf) {
    |                   ^^^^^^ not found in this scope

error: cannot find macro `cfg_if` in this scope
    --> /Users/xxx/.cargo/registry/src/github.com-1ecc6299db9ec823/log-0.4.17/src/lib.rs:1654:1
     |
1654 | cfg_if! {
     | ^^^^^^
     |

takahser avatar Nov 11 '22 12:11 takahser

@takahser Thanks for your time.

1

This is related to:

  • https://github.com/hack-ink/subalfred/issues/71
  • https://github.com/hack-ink/subalfred/pull/259.

In the past, users need to run subalfred state export --at <hash>. With this update, they could use a block number directly.

2

I couldn't produce it locally. Also, you could check the CI.

Ah, I know. You updated your Rust toolchain accidentally and didn't perform a cargo clean after that. You were trying to compile the old cache with a new toolchain.

error[E0514]: found crate `cfg_if` compiled by an incompatible version of rustic
..
    = help: please recompile that crate using this compiler (rustc 1.66.0-nightly (a37499ae6 2022-09-18)) (consider running `cargo clean` first)

So, these errors/warnings are irrelevant to Subalfred.

aurexav avatar Nov 11 '22 13:11 aurexav

Congratulations on completing the first milestone of this grant! As part of the Grants Program, we want to help grant recipients acknowledge their grants publicly. To that end, we’ve created a badge for projects that successfully deliver their first milestone. Note that it must only be used within the context of the delivered work, so please do not display it on your team or project's homepage unless accompanied by a short description of the grant.

Furthermore, you're now welcome to announce the grant publicly. Please remember to observe the foundation’s guidelines in doing so. In case you haven't done so yet, you may also reach out to [email protected] for feedback on your announcement and cross-promotion.

Thank you for your contribution and good luck with the remaining milestones, if any! As usual, please let us know if you run into any delays by leaving a comment on the application PR, or directly submitting an amendment.

github-actions[bot] avatar Nov 11 '22 15:11 github-actions[bot]

Payment received. Checked.

Thanks!

aurexav avatar Nov 19 '22 09:11 aurexav