Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Crowdfund parachain deposit #315

Merged
merged 49 commits into from Sep 17, 2019
Merged

Crowdfund parachain deposit #315

merged 49 commits into from Sep 17, 2019

Conversation

gavofyork
Copy link
Member

No description provided.

@gavofyork gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jul 8, 2019
@shawntabrizi shawntabrizi self-assigned this Jul 12, 2019
runtime/src/crowdfund.rs Outdated Show resolved Hide resolved
runtime/src/crowdfund.rs Outdated Show resolved Hide resolved
@gavofyork
Copy link
Member Author

@shawntabrizi would be good to get tests in for funds that span multiple auctions and funds that begin prior to any auction.

@shawntabrizi
Copy link
Contributor

@shawntabrizi would be good to get tests in for funds that span multiple auctions and funds that begin prior to any auction.

Yes, I can do this asap.

Still have to write some new follow up tests though
@shawntabrizi
Copy link
Contributor

my general feeling is that runtime correctness checks should only be conducted when there's no possibility of making a static correctness check. in cases where it's demonstrably impossible or impractical for an error to be triggered then i'm against littering the code.

This makes sense to me, but now I struggle to understand how best to teach a concept like this to new runtime developers. Will follow up with you offline.

does it also do exactly the right thing when the fund happens over multiple auctions?

You are right it does not work over multiple auctions. The updated logic you introduced makes sense to me, and handles this situation well. I will write tests to check it.

@shawntabrizi
Copy link
Contributor

@gavofyork a crowdfund with an end date longer than a single auction seems to work with the last test I added. However, from one action end to the next auction, there must be a contribution to the fund in order for it to be added to NewRaise again, and submit a bid.

Is this okay?

@gavofyork
Copy link
Member Author

The decision on when/whether to place an item in NewRaise should be tested thoroughly... Does it check when the fund has a bid outside of an ending period, then in a later auction, also outside the ending period, a winning bid?

@gavofyork
Copy link
Member Author

ok - i'm reasonably happy with it.

@gavofyork
Copy link
Member Author

@shawntabrizi once you're happy with it, give it a final review and we can merge.

@shawntabrizi
Copy link
Contributor

Note: Right now only create has a transaction weight. Could need a follow up after doing analysis on the functions.

@gavofyork gavofyork merged commit 577a5a2 into master Sep 17, 2019
@gavofyork gavofyork deleted the gav-crowdfund branch September 17, 2019 10:22
tomusdrw added a commit that referenced this pull request Mar 26, 2021
imstar15 pushed a commit to imstar15/polkadot that referenced this pull request Aug 25, 2021
…aritytech#315)

* Block announce validation should use the correct `Validation` result

The error variant is just for internal errors and we need to return
`Failure` always when the other node send us an invalid statement.

* Update network/src/lib.rs

Co-authored-by: Sergei Shulepov <sergei@parity.io>

Co-authored-by: Sergei Shulepov <sergei@parity.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants