Liberland Milestone 2 Delivery

[DorianSternVukotic]

Milestone Delivery Checklist

• The milestone-delivery-template.md has been copied and updated.
• The invoice form 📝 has been filled out for this milestone.
• This pull request is being made by the same account as the accepted application.
• In case of acceptance, the payment will be transferred to the BTC/ETH/fiat account provided in the application.
• The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: Liberland grant application

[DorianSternVukotic]

We will add the article soon

[stojanov-igor]

Hi @DorianSternVukotic

I have started an external evaluation for your delivery. The initial comments you can find here: #829 .I will update it further once you deliver the article.

[DorianSternVukotic]

Hi @DorianSternVukotic

I have started an external evaluation for your delivery. The initial comments you can find here: #829 .I will update it further once you deliver the article.

Hi @JosephKnecht-lab
This was explained in the notes and communicated with the grant team. You had questions about missing metaverse registry pallet. Once we started developing, it turned out that all data handling regarding registries can be handled by a slightly modified nfts pallet, so we use that to handle the data. What was missing was registry admin stuff, which has been developed in the form of the Offices pallet. Coordinates are validated off-chain (it really makes no sense to do it on-chain), and of course, visualized off-chain. So, in this case the deliverable isnt a 'pallet' itself but a feature including many parts.

So, Land registry pallet is
LandRegistryOffice instance of the Office pallet -> Managing collection of the modified nfts pallet -> displayed offchain

Metaverse Land Registry is not a pallet itself but
MetaverseLandRegistryOffice instance of the office pallet -> Managing collection of the modified nfts pallet -> Data prepared for metaverse through middleware server -> Displayed in the metaverse ( See testing guide for details )

You can check it out on Liberland testnet or mainnet of course.

[stojanov-igor]

Hi @DorianSternVukotic,

Thank you for the clarification. I have updated my evaluation and these are my concluding remarks.

Based on my evaluation, you have not provided all the deliverables as described in you application. Thus I have to reject this delivery. My advise is to update your initial application with the exact deliverables that you are delivering for this milestone, before it can be accepted.

Furthermore, a lot of your pallets are derivative work from already existing pallets (such as registry, nfts) and I would expect more original work for grant submission.

Note that this evaluation is not binding since I do not work of Web3 Foundation, so the Web3 team can accept your delivery in its current state.

[takahser]

@DorianSternVukotic FYI - I merged #829 into a seperate branch, dedicated to this delivery. I'm going to take over from here, see also this comment. I'm going to give you more feedback soon.

[DorianSternVukotic]

Thanks for the update but i feel like in your review you didnt take into consideration the explaination or the testing guide.

The issue raised about the land registry is that its almost an exact fork of the nfts pallet. I can see how that is confusing, but let me try to clear it up. The requirement was to build nft-fueled Land Registry. We built a completely new offices pallet, which handles the inner workings, administration and permissioning of a Registry, adapting how real-life registries work to blockchain environment. This Registry office in turn manages an nft collection. I dont think anyone expected us to build a custom nft engine for this, as off the shelf nft solutions are sufficient. The Land registry is an instance of a (new) office pallet managing the modified NFT pallet collection. Land registry therefore, is more than just a slightly modified nfts pallet as you described it.

Regarding lack of hardcoded geocordination logic, there are many checks that are required for a land registry that are impossible to do on-chain, so hardcoding some checks is not useful for anyone using the land registry, and would just reduce the number of use cases our pallet could be used for. We can easily add geocoordinate checks in the pallet if its required for the grant, but that would be a feature developed solely for grant acceptance and not for real world use.

Regarding the metaverse integration, it works which you can check by following the testing guide. I understand that technically the grant specified it is in the form of a dedicated pallet, and that the "proper" solution would be to modify the proposal to delete that word, but similarly to the land registry pallet, metaverse registries integration is a combination of our original offices pallet, the nfts pallet, and code on the metaverse. We have achieved the integration of registered metaverse land and other nfts in the metaverse. If its required for the grant, we could add some hardcoded metaversey logic and throw it together in the shape of a pallet, but again, it would be writing code that is useless for actual applications, and we were advised not to do that and focus instead on the requirements.

I agree that the requirements should be more defined, and i understand that it makes this delivery a bit confusing.
We are available to discuss solutions, and will make the neccessary modifications to either the application or the delivery as needed.

[takahser]

@DorianSternVukotic thanks for the update. I identified some issues when building and running the tests, see the updated evaluation.

Regarding the issues raised by @JosephKnecht-lab on the landowner pallet and the metaverse integration pallet, kindly address the build issues highlighted in the updated evaluation first. This will enable me to perform some smoke testing on your delivery before providing further feedback. Doing so will aid in comprehending the points presented by @JosephKnecht-lab and the counter-arguments you provided.

[DorianSternVukotic]

@takahser

Sorry about that.

Background:
Using latest Rust requires polkadot-v0.9.39, see upstream issue:
paritytech/substrate#13636 . We're on
polkadot-v0.9.37, so we need a bit older Rust. Updated docs and scripts

Read README and re run everything from scratch on DEVELOP branch
https://github.com/liberland/liberland_substrate/tree/develop

OR Quick fix:

Run rustup default nightly-2023-01-01 && rustup target add wasm32-unknown-unknown and rerun tests.

[takahser]

@DorianSternVukotic this fixes the previously failing test but now the "temp_base_path_works" test is failing, see my updated eval. Am I still missing something?

[DorianSternVukotic]

@takahser
This test checks the "--tmp" command line option. If its used, the node should create a temporary database directory and delete on exit. It looks like for for you it doesnt delete it or there's some kind of race condition (which could be file-system and OS dependant).
We didnt touch relevant code at all, so my guess is that this is broken upstream as well, but I dont have Mac to verify. Lets test on Linux for now, and we sync to upstream semi-regularly

[takahser]

@DorianSternVukotic thanks for your patience and your fixes. LGTM now, feel free to have a look at the updated eval.