feature: get_children_ids_recursive

[Tguntenaar]

Resolves #11

@frol @ailisp if I add more posts to the test the gas limit exceeds. How can I add a significant number of posts and prevent exceeding the gas limit in the 'test_get_children_ids_recursive' function?

[ailisp]

@Tguntenaar You can use this method https://docs.rs/near-sdk/latest/near_sdk/test_utils/struct.VMContextBuilder.html#method.prepaid_gas

to

        VMContextBuilder::new()

For example:

        VMContextBuilder::new()
            .predecessor_account_id(signer.try_into().unwrap())
            .is_view(is_view)
            .prepaid_gas(Gas(100_000_000_000_000)) // add this, 100 T gas
            .build()

[ailisp]

@Tguntenaar 300T gas is the maximum allowed on near blockchain, so check if you hit a infinite or too expensive recursion

[Tguntenaar]

It seems that by default the maximum of 300 TGas is attached. Should I create a second context? Seems to work but feels like I missing another solution.

@ailisp @frol , it isn't solely related to the recursive function itself. Even excluding it from the unit test, attempting to add just a few posts still results in exceeding the gas limit.

I will replace the recursive function with an iterative version to be sure.

The thing with the VMContextBuilder is that the unit test is meant to test a small part of the code. Namely the one function it should test. 300 Tgas is assigned when the context is being build. Usually this would be enough to test a few contract calls, but the it seems that add_post is more gas intensive. It doesn't account for the fact that in order to test this get_all_children_ids function I need to call add_post a few times.

I could be wrong and missing something in which case please let me know.

PS So that's why I just made two more contexts to reset the gas to the limit. I think it would be a better solution to refactor the contextbuilder a bit to be able to add gass to the context in contract calls.

Another thing is we could limit the amount get_all_children_ids counts and just display 99+ or something similar.

[Tguntenaar]

@frol @ailisp I wanted to ping either of you regarding this comment. I'm awaiting your response for a 👍 .

EDIT never mind I forgot it was a holiday sorry!

[ailisp]

@Tguntenaar good findings! In fact, you can create another context object after gas use up (to get another 300 T gas). So this way you can create arbitrary number of posts.

Another thing is we could limit the amount get_all_children_ids counts and just display 99+ or something similar.

This is a brilliant idea! We would want to figure out the number "99" or more here. Could you experiment it a bit, for example, get_all_children_ids with 10 child posts, 20 child posts, 100 child posts. And count the gas usage. Then further add 10 posts each with 10 children posts. We can estimate a rough gas usage to get_all_children_ids in this case.

Say, if the result is quite little gas even for hundreds of post, we are not bothered to limit the number. Otherwise you can figure out a safe limit like "99" or so.

[Tguntenaar]

Alright I will find out the gas price based on the depth.

[Tguntenaar]

I'm closing this PR because @frol made this comment: I suggest we don't invest any more time into old style posts / notifications / nested comments / kanban boards since proposals and announcements will fix many of the issues by design or will require rethinking of the approach.