Create starlay_finance-milestone_1.md

[Chidasan]

No description provided.

[HCastano]

Do you have any sort of recommend workflow for testing the the functionality works?

It would also be nice to have some sort of video (e.g using Loom) to show that as well

[deepsea514]

You can test functionalities using e2e tests already made.
E2E test of pool

Run swanky-node on local side and run this command.

yarn compile
yarn test:single tests/Pool.spec.ts

[Chidasan]

Hi @HCastano
I'm following up on this milestone-1. Could you test the above functionalities and let us know if anything we need to change.

[HCastano]

Hey @Chidasan, yes sorry we'll take a look in the next couple of days 🙇

[Chidasan]

Hey @Chidasan, yes sorry we'll take a look in the next couple of days 🙇

Thank you for your response. We understand that you have a busy schedule and we appreciate your commitment to review our proposal. We look forward to hearing from you soon.

[HCastano]

Hey, I took a brief look through the code and have a few comments and questions.

• On the Milestone application you mentioned that the code would be licensed as Apache 2, but the current license you have is AGPL.
• In the milestone application you mentioned that you'd include written and video tutorials on how to use Starlay. I don't see any video tutorials.
• Can you clarify what the legal entity behind the Starlay project is?
• The version of rustc specified in the rust-toolchain.toml file is a bit old, why are you still using this version?
• You're using OpenBrush's ZERO_ADDRESS in a lot of places. I'd recommend steering away from this and using something like Rust's Option instead. There are some security implications of using the zero address since it has a known private key.
• You're also using an old version of OpenBrush, I'd recommend updating if you can.
• You're missing a lot of inline documentation. At minimum I'd expect all public APIs to be documented using RustDoc. Take a look through the Rust API guidelines for more on that topic, as well as other ways to improve your code.
• You should consider running Clippy and RustFmt as part of your
• You can clean a few things up in your CI config. For example, you don't need to install binaryen manually anymore. Similarly, you're not making use of the linting, do you don't need the dylint packages.
• You should be running the TypeScript E2E tests as part of your CI.
• You should make use of the ink_e2e crate to do E2E testing in Rust. This would be good to ensure that functionality works independent of the TS tests.
• You should have a Dockerfile available for easy sharing and testing of the system.

---

I also tried to run the project but I couldn't get past the yarn compile step. I'm trying on the following commit: 0c5a41b5.

$ yarn --version
1.22.19
$ node --version
v20.2.0

Running yarn && yarn compile results in the following:

workspace: /home/starlay/Cargo.toml
    Updating crates.io index
    Updating git repository `https://github.com/727-Ventures/openbrush-contracts`
    Updating git repository `https://github.com/727-ventures/pallet-assets-chain-extension`
    Updating git repository `https://github.com/727-Ventures/obce`
    Finished release [optimized] target(s) in 1.12s
======== Compiled weth ========                                                               8:48:55 PM
======== Compiled all contracts ========                                                      8:48:55 PM
$ typechain-polkadot --in artifacts --out types --plugins typechain/plugins
Succesfully loaded plugins:  U256TypesReturnsOverride, WrappedU256DataOverride
SyntaxError: Expected double-quoted property name in JSON at position 24107
    at JSON.parse (<anonymous>)
    at WrappedU256DataOverridePlugin.generate (/home/starlay/typechain/plugins/wrappedU256.data.plugin.ts:30:23)
    at TypechainPolkadot.run (/home/starlay/node_modules/@727-ventures/typechain-polkadot/src/types/typechain.ts:61:18)
    at async main (/home/starlay/node_modules/@727-ventures/typechain-polkadot/index.ts:98:2)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command

Let me know if you need more info about my setup.

[HCastano]

Can you add GitHub permalinks here instead? Or have a tag for each milestone submission? As you push more code to the repo these links get out of sync and don't line up anymore

[Chidasan]

Thank you for your insightful review. We will carefully consider each of your points and respond accordingly. However, some responses may be challenging due to constraints related to time and manpower. We’d appreciate your understanding as we navigate through these issues, and would like to propose addressing certain parts in the next milestone.

You’re using OpenBrush’s ZERO_ADDRESS in a lot of places. I’d recommend steering away from this and using something like Rust’s Option instead.

There are some security implications of using the zero address since it has a known private key. We understand the vulnerabilities. use-ink/ink#1255 It’s just something we haven’t released, so I’d like to make it a required item for the next milestone.

You should be running the TypeScript E2E tests as part of your CI. You should make use of the ink_e2e crate to do E2E testing in Rust. This would be good to ensure that functionality works independent of the TS tests.

Since we are currently using swanky-node, we need to change the infrastructure of E2E itself. The TS E2E itself is already in place, and the quality implementation itself is at least satisfactory, so incorporation into CI and selection of tools to be used is a type of improvement. Let this also be a necessary element of the next milestone.

In conclusion, while we intend to address some of the points mentioned, we will provide you with an update on our progress by the end of this week. Please note that some of the issues highlighted might not be addressed in the immediate term due to the constraints mentioned earlier. However, we aspire to tackle them in the forthcoming milestone as much as possible.

[artat9]

Hi, @HCastano , I'm also working on this project.

The version of rustc specified in the rust-toolchain.toml file is a bit old, why are you still using this version?

openbursh-contracts currently supports old versions of rustc, so we don't use the latest one.

[un-ast]

Hi, @HCastano , I'm also working on this project.

You're missing a lot of inline documentation. At minimum I'd expect all public APIs to be documented using RustDoc. Take a look through the Rust API guidelines for more on that topic, as well as other ways to improve your code.

I followed up on this one.
starlay-finance/starlay-protocol-wasm#118

We added comments focusing on the following.

• The interfaces of contracts (i.e., trait)
• Storages of Contract (i.e., Struct)
• Event Definitions

Please let me know if there are any concerns.

[Chidasan]

Hi, @HCastano

Here are the updates we've made so far. Regrettably, we've found that the completion of our task is taking longer than initially anticipated. This is mainly due to the complexities involved with upgrading our third-party tools. Specifically, the discrepancy between the version we initially developed with and the version we intend to upgrade to is substantial, thus requiring more time to adequately address and resolve any compatibility issues.

Given these circumstances, could we move the remaining tasks below to our next milestone(1-2)? We believe this could allow us to manage the upgrade process more effectively.

Completed tasks

On the Milestone application you mentioned that the code would be licensed as Apache 2, but the current license you have is AGPL.

Changed it. https://github.com/starlay-finance/starlay-protocol-wasm/blob/main/LICENSE.md

In the milestone application you mentioned that you'd include written and video tutorials on how to use Starlay. I don't see any video tutorials.

The link provided below gives access to our most recent collection of articles and videos. Please note, our application is currently running on the testnet, which has a different URL from the mainnet version. As such, we have chosen to refrain from updating these materials on our gitbook page until the mainnet launch. Once we transition to the mainnet, we will promptly update our gitbook page with the relevant information and resources.

https://unruly-innovation-bff.notion.site/Educational-articles-and-videos-5a9fd96d676a47fdbd411c07e0857f9e?pvs=4

Can you clarify what the legal entity behind the Starlay project is?

Asynmatrix Pte. Ltd.
Should we write this info on LICENSE? or README?

You're missing a lot of inline documentation. At minimum I'd expect all public APIs to be documented using RustDoc. Take a look through the [Rust API guidelines](https://rust-lang.github.io/api-guidelines/documentation.html#all-items-have-a-rustdoc-example-c-example) for more on that topic, as well as other ways to improve your code.

Done

You should consider running Clippy and RustFmt as part of your

Done

You can clean a few things up in your CI config. For example, you don't need to install binaryen manually anymore. Similarly, you're not making use of the linting, do you don't need the dylint packages.

Done

You should have a Dockerfile available for easy sharing and testing of the system.

Done

Can you add GitHub permalinks here instead? Or have a tag for each milestone submission? As you push more code to the repo these links get out of sync and don't line up anymore

Done

Remaining tasks

The version of rustc specified in the rust-toolchain.toml file is a bit old, why are you still using this version?

You're using OpenBrush's ZERO_ADDRESS in a lot of places. I'd recommend steering away from this and using something like Rust's Option instead. There are some security implications of using the zero address since it has a known private key.

You're also using an old version of OpenBrush, I'd recommend updating if you can.

You should be running the TypeScript E2E tests as part of your CI.

You should make use of the ink_e2e crate to do E2E testing in Rust. This would be good to ensure that functionality works independent of the TS tests.

[HCastano]

@Chidasan sorry for the late response, I've been busy with Decoded related things. I'll re-review this next week 🙏

[HCastano]

Thanks for addressing some of my previous comments.

Here's some feedback from another review round.

For the license:

• You'll want to switch the LICENSE text to be your registered entity, not "Starlay"
• You may also want to have a license header in each file of your project, but this
  is not a hard requirement from
• https://www.apache.org/foundation/license-faq.html#Apply-My-Software

For the TS tests:

• Adding support for the E2E tests to run as part of the CI in the future milestone is fine

For the videos:

• I can't access the Notion page with the videos. Can you either grant open access or post
  them to YouTube or something temporarily?

For the Dockerfile:

• Thanks for adding the Devcontainer and Dockerfile. I'd take this one step further though and have everything (Rust + JS + Contracts Node) bundled in a single Dockerfile. This would then allow reviewers to really quickly get up and running. One approach would be to base your image off the image provided by Swanky. This would be fine to do in a later milestone though.

For the rust-toolchain file

• It's missing the rust-src component (so components = [..., "rust-src"]).

Lastly, I didn't manage to get your TypeScript tests passing this time either.

Test Suites: 4 failed, 2 passed, 6 total
Tests:       25 failed, 1 skipped, 76 passed, 102 total
Snapshots:   0 total
Time:        73.516 s
Ran all test suites matching /.spec.ts$/i.

I'm running the build and tests inside of the following Swanky Dockerfile: ghcr.io/astarnetwork/swanky-cli/swanky-base:swanky3.0.4_v2.0.3.

Yarn Version: 1.22.19
Node Version: v20.2.0
Swanky Node Version: 1.6.0-e5e6b8f914b

Having some pinned versions and more verbose instructions in the project README would be helpful to ensure that I'm running everything correctly.

[HCastano]

For future reviewers, here are some of the related TS E2E tests:

• Borrow
  • https://github.com/starlay-finance/starlay-protocol-wasm/blob/f8465b9e79b7566610663fd5f2971a9c0eeac8f9/tests/Pool.spec.ts#L381
• Mint
  • https://github.com/starlay-finance/starlay-protocol-wasm/blob/f8465b9e79b7566610663fd5f2971a9c0eeac8f9/tests/Pool.spec.ts#L111
• Redeem
  • https://github.com/starlay-finance/starlay-protocol-wasm/blob/f8465b9e79b7566610663fd5f2971a9c0eeac8f9/tests/Pool.spec.ts#L206
• Repay Borrow
  • https://github.com/starlay-finance/starlay-protocol-wasm/blob/f8465b9e79b7566610663fd5f2971a9c0eeac8f9/tests/Pool.spec.ts#L472

[Chidasan]

@HCastano
Thank you for taking the time to provide detailed feedback once again.

We' have updated the licence, the rust-toolchain, and README. You can find all these modifications at the following link: starlay-finance/starlay-protocol-wasm#122

For the videos:
I can't access the Notion page with the videos. Can you either grant open access or postthem to YouTube or something temporarily?

I have enabled index searching, even though open access was previously granted.
https://unruly-innovation-bff.notion.site/Informative-articles-and-instructional-videos-5a9fd96d676a47fdbd411c07e0857f9e?pvs=25
I've also uploaded the videos to Google Drive. Please take a look at them.
https://drive.google.com/drive/folders/1xtHuAlB516XPGs6SH1sHs0Ze6Uf6PFHC?usp=sharing

Lastly, I didn't manage to get your TypeScript tests passing this time either.

As I commented two weeks ago, we're facing difficulties in passing the test due to issues with third-party dependencies. The discrepancy between the version we developed with and the version we're upgrading to is causing significant compatibility issues.

As previously proposed, due to these third-party dependency issues, it would be extremely beneficial if we could shift these tasks to the next or even the following milestone. We appreciate your understanding and flexibility regarding this issue.

In light of all these updates, I believe we've addressed all the tasks for milestone 1-1. If our understanding aligns with yours, we would appreciate if we could proceed to the next milestone, milestone 1-2. Thank you for your understanding and flexibility regarding these matters.

[HCastano]

If I'm being honest I don't want to approve this milestone without passing tests. There's no other easy way for me to verify that behaviour is correct. The alternatives I have are to dig through the code much more deeply, or manually deploy this somewhere and test it out there (but even then, I'd have to know more about the intended workflows).

That said, since it does look like all the functionality is there, and since this has been pending for a while I don't think it's fair to keep you waiting. I'll approve on the condition that you get the tests passing for the next milestone.

As a development team you need to treat your tests like first class citizens and ensure that they're always working. It gives you more confidence as you move forward with development.

[Chidasan]

Thank you for your understanding and approval under these circumstances. We're completely aligned with you on the importance of having passing tests as a measure of our software's correctness. It is an unfortunate situation that external factors beyond our control have prevented us from providing you with a passing test suite in this instance.

As I mentioned before, the current issues are primarily due to version inconsistencies between the tools used during our initial Testnet development in April, and their updated versions. These updates are unfortunately incompatible with our development environment and are causing failures in our tests. Specifically, we're facing two major issues:

ink! : This tool’s recent update has introduced compatibility issues with our Testnet version’s codebase. The lack of backward compatibility is causing errors during our tests.
openbrush : This tool, essential for our development workflow, are not supported by our existing codebase. The discrepancy here is also leading to test failures.

We’re actively talking with the Openbrush core team for the updates (and they said they’d release new version this week) and working on strategies to adapt to these changes. Meanwhile, we believe it’s most prudent to wait for further updates that we, as the Starlay team, are not responsible for initiating. Once these updates are made, we will be able to align our codebase effectively and solve the current testing failures

Thank you once again for your flexibility in this situation.

[TtomaS7]

Hey, Starlay team!
We are aware about your interest in an audit from our ink!ubator Auditors.
To facilitate this process, could you kindly create a brief document, spanning 1-2 pages, outlining your project and the specific contracts you'd like audited? Please include the links to these contracts on GitHub for easy access. Furthermore, if you've undergone any audits previously, please note this in the document and attach the respective report.

Thank you for your cooperation

[Chidasan]

Hi @TtomaS7 @HCastano and @DoubleOTheven

I noticed that this pull request has been in the "waiting for payment" phase for a considerable amount of time now.

We understand that there might be various reasons for the delay, but we would greatly appreciate any updates or feedback you could provide at this time.

Thank you for your attention to this matter. We look forward to hearing from you soon.

[TtomaS7]

Hey @Chidasan!
Thank you for the heads up. The Election of the curation team Proposal is still ongoing, but it will pass in 7 days, and we can start initiating the payments. We will share the instructions regarding how to create a Child bounty and request payment soon.
Thanks again!

[TtomaS7]

Hey team!

Here's the form for payment details: https://forms.gle/BUCKfygkiEvpdYna6.
Once you've filled it out, please let me know and be sure to specify what it pertains to; you can include the link to this PR delivery! Further details can be provided under the section: "Please describe which Milestone you have completed and attach the link to the Milestone delivery."

Should you have any questions, don't hesitate to ask. Thank you so much!

Important Note: PLEASE VERIFY YOUR ADDRESS, WHERE YOU WILL RECEIVE THE FUNDS, BEFORE FILLING OUT THE FORM!
For guidance on how to set up identity, refer to this guide.

[Chidasan]

@TtomaS7

Thank you for providing the form and the detailed instructions. We have submitted our payment details and specified it pertains to the submission of Milestone 1 and Milestone 1-2, including the link to the PR delivery in the form.

We've double-checked our address for receiving the funds and ensured it's accurate before submitting the form.

Thanks for your support as always!

[TtomaS7]

Hey!
It seems that your address is not verified, please check the address and fill the form again

[Chidasan]

Hi @TtomaS7,

We've now registered our identity as "Starlay Finance" and have made the necessary corrections to the form and resubmitted it. You can verify our updated address: https://explorer.polkascan.io/polkadot/account/12soSZUb7UbsC8iLrTy6dUVkKCfNvQxMMSRtvZJkfQL1BPRX.

[TtomaS7]

thank you!

[TtomaS7]

The payment sent!

https://polkadot.subsquare.io/treasury/child-bounties/19_461
You can claim it in 7 day, thank you!