cadence icon indicating copy to clipboard operation
cadence copied to clipboard

Published 20 hours ago • verified publisher icon onflow

add a `clear` method to delete saved data in storage

Open dsainati1 opened this issue 3 years ago • 7 comments

Closes #1245

Adds a new method to the AuthAccount object:

fun clear(_ path: Path): Bool

This function deletes whatever is present at the specified path, destroying any resources, and returns whether anything was present originally.

  • [X] Targeted PR against master branch
  • [X] Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • [X] Code follows the standards mentioned here
  • [X] Updated relevant documentation
  • [X] Re-reviewed Files changed in the Github PR explorer
  • [X] Added appropriate labels

dsainati1 avatar Nov 17 '21 21:11 dsainati1

Codecov Report

Merging #1253 (2ee3fb9) into master (81c63bf) will increase coverage by 0.01%. The diff coverage is 94.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1253      +/-   ##
==========================================
+ Coverage   77.60%   77.61%   +0.01%     
==========================================
  Files         274      274              
  Lines       35233    35261      +28     
==========================================
+ Hits        27341    27369      +28     
  Misses       6805     6805              
  Partials     1087     1087
Flag Coverage Δ
unittests 77.61% <94.59%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
runtime/interpreter/interpreter.go 89.19% <89.47%> (+0.06%) :arrow_up:
runtime/interpreter/account.go 93.16% <100.00%> (+0.17%) :arrow_up:
runtime/sema/authaccount_type.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 81c63bf...2ee3fb9. Read the comment docs.

codecov-commenter avatar Nov 17 '21 21:11 codecov-commenter

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit 81c63bfef73cd4874d05b6274dbd2402a0a0b788 The command go test ./... -run=XXX -bench=. -shuffle=on -count N was used. Bench tests were run a total of 7 times on each branch.

Results

old.txtnew.txt
time/opdelta
QualifiedIdentifierCreation/One_level-23.17ns ± 4%3.30ns ± 2%+4.14%(p=0.008 n=7+6)
ParseArray-222.6ms ± 1%23.1ms ± 2%+1.88%(p=0.005 n=6+7)
RuntimeStorageWriteCached-2169µs ±43%143µs ± 2%~(p=0.731 n=7+6)
RuntimeResourceDictionaryValues-216.7ms ± 3%16.4ms ± 3%~(p=0.534 n=7+6)
RuntimeFungibleTokenTransfer-21.24ms ± 4%1.24ms ± 3%~(p=0.445 n=7+6)
ParseFungibleToken-2469µs ± 2%475µs ± 2%~(p=0.165 n=7+7)
ParseInfix-224.6µs ± 2%24.9µs ± 4%~(p=0.456 n=7+7)
ParseDeploy/decode_hex-21.44ms ± 4%1.43ms ± 3%~(p=1.000 n=7+7)
QualifiedIdentifierCreation/Three_levels-2161ns ± 2%163ns ± 4%~(p=0.386 n=6+7)
CheckContractInterfaceFungibleTokenConformance-2168µs ± 3%171µs ± 5%~(p=0.128 n=7+7)
ContractInterfaceFungibleToken-247.4µs ± 1%47.8µs ± 2%~(p=0.318 n=7+7)
InterpretRecursionFib-22.71ms ± 3%2.68ms ± 3%~(p=0.945 n=7+6)
NewInterpreter/new_interpreter-21.16µs ± 5%1.16µs ± 2%~(p=0.781 n=7+7)
NewInterpreter/new_sub-interpreter-22.11µs ± 3%2.10µs ± 3%~(p=0.779 n=7+7)
ParseDeploy/byte_array-235.0ms ± 3%34.3ms ± 2%−2.15%(p=0.038 n=7+7)
 
alloc/opdelta
RuntimeFungibleTokenTransfer-2233kB ± 0%234kB ± 0%+0.40%(p=0.001 n=7+7)
RuntimeResourceDictionaryValues-24.34MB ± 0%4.34MB ± 0%+0.02%(p=0.001 n=7+7)
RuntimeStorageWriteCached-283.7kB ± 0%83.7kB ± 0%~(p=0.294 n=6+7)
QualifiedIdentifierCreation/One_level-20.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-264.0B ± 0%64.0B ± 0%~(all equal)
CheckContractInterfaceFungibleTokenConformance-266.4kB ± 0%66.4kB ± 0%~(p=1.000 n=7+7)
ContractInterfaceFungibleToken-226.7kB ± 0%26.7kB ± 0%~(all equal)
InterpretRecursionFib-21.21MB ± 0%1.21MB ± 0%~(p=0.385 n=7+6)
NewInterpreter/new_interpreter-2680B ± 0%680B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-21.06kB ± 0%1.06kB ± 0%~(all equal)
 
allocs/opdelta
RuntimeFungibleTokenTransfer-24.42k ± 0%4.43k ± 0%+0.06%(p=0.001 n=6+7)
RuntimeResourceDictionaryValues-2108k ± 0%108k ± 0%+0.00%(p=0.005 n=7+7)
RuntimeStorageWriteCached-21.42k ± 0%1.42k ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-22.00 ± 0%2.00 ± 0%~(all equal)
CheckContractInterfaceFungibleTokenConformance-21.07k ± 0%1.07k ± 0%~(all equal)
ContractInterfaceFungibleToken-2458 ± 0%458 ± 0%~(all equal)
InterpretRecursionFib-225.0k ± 0%25.0k ± 0%~(all equal)
NewInterpreter/new_interpreter-211.0 ± 0%11.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-231.0 ± 0%31.0 ± 0%~(all equal)
 

github-actions[bot] avatar Nov 17 '21 21:11 github-actions[bot]

@dete Any thoughts?

turbolent avatar Nov 19 '21 21:11 turbolent

@dsainati1 Should we maybe consider this change for Stable Cadence?

turbolent avatar Oct 04 '22 23:10 turbolent

@dsainati1 Should we maybe consider this change for Stable Cadence?

Seems to me like based on the discussion on https://github.com/onflow/cadence/pull/1252, the sentiment is against these two methods, the argument being that any operation that destroys a resource should be explicit; having methods that can implicitly destroy resources is not a desirable feature for the language.

However I would be willing to put this to a FLIP if it's something we want to push for anyways.

dsainati1 avatar Oct 05 '22 14:10 dsainati1

@dsainati1 Sounds good! Added it to the agenda of the next LDM, maybe we can decide there together if we want to close this and the other related PR, or open a FLIP for them

turbolent avatar Oct 05 '22 18:10 turbolent

Put on-hold for now, see https://github.com/onflow/cadence/blob/master/meetings/2022-10-11.md#additional-storage-api-functions

turbolent avatar Oct 12 '22 18:10 turbolent

Seems like this feature is not wanted; closing.

dsainati1 avatar Jan 13 '23 18:01 dsainati1