feat: thanks and much gratitude! #282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
we always only want to use accountIDs other lookups are not wanted
|
/bench runtime spiritnet-runtime crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs ResultsERROR: Unable to commit file ./runtimes/spiritnet/src/weights/crowdloan.rs |
|
/bench runtime spiritnet-runtime crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs ResultsERROR: Unable to commit file ./runtimes/spiritnet/src/weights/crowdloan.rs |
|
/bench runtime spiritnet-runtime crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs Results |
…hmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs
|
/bench runtime peregrine-runtime crowdloan |
|
Error running benchmark: aw-thanks stdoutCaught exception in benchmarkRuntime: TypeError: Cannot read properties of undefined (reading 'benchCommand') |
|
/bench runtime peregrine crowdloan |
|
Benchmark Runtime Substrate Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs Results |
…hmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs
|
/bench runtime pallet crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/crowdloan/src/default_weights.rs --template=.maintain/weight-template.hbs Results |
…hmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/crowdloan/src/default_weights.rs --template=.maintain/weight-template.hbs
|
/bench runtime spiritnet-runtime crowdloan |
pallets/crowdloan/src/lib.rs
Outdated
| let gratitude = Self::split_gratitude_for(&receiver)?; | ||
| Self::ensure_can_send_gratitude(&gratitude)?; | ||
|
|
||
| // *** No failure beyond this call! The contributor was removed. *** |
There was a problem hiding this comment.
You cant just write no failure and then have a bunch of ? after this.
You should write an expect statement with a proof of why a call will not fail.
Otherwise, you should capture the result, saying that if something does fail, better to do nothing than to modify storage and then return error.
pallets/crowdloan/src/lib.rs
Outdated
|
|
||
| // Transfer the free amount. Should not fail since checked we | ||
| // ensure_can_withdraw. | ||
| CurrencyOf::<T>::transfer(&reserve.free, &receiver, free, ExistenceRequirement::AllowDeath)?; |
There was a problem hiding this comment.
Suggested change
| CurrencyOf::<T>::transfer(&reserve.free, &receiver, free, ExistenceRequirement::AllowDeath)?; | |
| CurrencyOf::<T>::transfer(&reserve.free, &receiver, free, ExistenceRequirement::AllowDeath) | |
| .expect("cannot fail since we checked ensure_can_withdraw"); |
This is one way you can approach it.
There was a problem hiding this comment.
Suggested change
| CurrencyOf::<T>::transfer(&reserve.free, &receiver, free, ExistenceRequirement::AllowDeath)?; | |
| let result = CurrencyOf::<T>::transfer(&reserve.free, &receiver, free, ExistenceRequirement::AllowDeath); | |
| debug_assert!(result.is_ok(), "free transfer failed after we checked in ensure_can_withdraw"); |
I would suggest this method if the worst case scenario is that someone doesn't get all of their funds, and that can be resolved after the fact.
There was a problem hiding this comment.
Could also use #[transactional] on this extrinsic if you are really worried that maybe some underlying assumption could be wrong.
There was a problem hiding this comment.
I went for trying the transfers and vesting and if there is any error we dispatch a specific error event. It feels the safest option. If there is a problem we get notified and can handle that manually. But that should not be possible after the checks.
a83a697
pallets/crowdloan/src/lib.rs
Outdated
|
|
||
| // Transfer the vested amount and set the vesting schedule. Should not fail | ||
| // since checked we ensure_can_withdraw. | ||
| CurrencyOf::<T>::transfer(&reserve.vested, &receiver, vested, ExistenceRequirement::AllowDeath)?; |
pallets/crowdloan/src/lib.rs
Outdated
| .checked_div(&BalanceOf::<T>::from(config.vesting_length)) | ||
| .unwrap_or(vested); | ||
| // vesting should not fail since we have transferred enough free balance. | ||
| VestingOf::<T>::add_vesting_schedule(&receiver, vested, per_block, config.start_block)?; |
There was a problem hiding this comment.
https://github.com/paritytech/substrate/blob/master/frame/vesting/src/lib.rs#L739
can_add_vesting_schedule should be called before add_vesting_schedule
There was a problem hiding this comment.
I thought the can_add_vesting_schedule would check that there is enough balance that can get vested. But that's not the case. So i added it to the checks. 👍🏻
|
/bench runtime spiritnet-runtime crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs Results |
…hmarks -- benchmark --chain=spiritnet-dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/spiritnet/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs
|
/bench runtime peregrine crowdloan |
|
Benchmark Runtime Substrate Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs Results |
…hmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./runtimes/peregrine/src/weights/crowdloan.rs --template=.maintain/runtime-weight-template.hbs
|
/bench runtime pallet crowdloan |
|
Benchmark Runtime Pallet for branch "aw-thanks" with command cargo run --quiet --release -p kilt-parachain --features=runtime-benchmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/crowdloan/src/default_weights.rs --template=.maintain/weight-template.hbs Results |
…hmarks -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=pallets/crowdloan/src/default_weights.rs --template=.maintain/weight-template.hbs
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes KILTprotocol/ticket/issues/1586
M
Checklist:
array[3]useget(3), ...)