Add support for resource-kinded fields in transaction roles #2262

turbolent · 2023-01-19T23:57:09Z

Work towards #2177

Description

Transaction roles may have resource-kinded fields.

Add dedicated support for allowing resource-kinded fields to get invalidated in a transaction's execute block, even though such invalidations are nested, which are not allowed in other contexts (composite destructors).

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

…quire invalidation of roles' members

turbolent · 2023-01-19T23:59:59Z

runtime/sema/checker.go

@@ -1366,32 +1366,13 @@ func (checker *Checker) recordResourceInvalidation(
 		)
 	}



The refactor of this function is probably best viewed in split-mode. Also, I first refactored the function a bit to simplify it in 7abf054 and to allow customization of the check what is allowed to a function in 48247ab.

turbolent · 2023-01-20T00:00:29Z

runtime/sema/check_transaction_declaration.go

+						return memberInfo.Member
+					}
+
+					return checker.accessedSelfMember(expression)


The default, like in https://github.com/onflow/cadence/pull/2262/files#diff-c34bee2125d8f3673160cc7b3c912540ad811b4622a66c3a2ed6c25fc94e1cb0R176 (jump is broken, need to scroll up a bit)

runtime/sema/checker.go

runtime/tests/checker/transactions_test.go

…fier of member expression

…d field

turbolent · 2023-01-27T20:24:14Z

@SupunS pushed some more tests and improvements, could you please have a final look? Thank you!

…ded-transaction-format-4

codecov · 2023-01-30T23:16:09Z

Codecov Report

Merging #2262 (7d32929) into feature/extended-transaction-format (dc837ee) will increase coverage by 0.00%.
The diff coverage is 97.22%.

@@                         Coverage Diff                          @@
##           feature/extended-transaction-format    #2262   +/-   ##
====================================================================
  Coverage                                77.79%   77.80%           
====================================================================
  Files                                      309      309           
  Lines                                    66164    66189   +25     
====================================================================
+ Hits                                     51474    51499   +25     
  Misses                                   12906    12906           
  Partials                                  1784     1784

Flag	Coverage Δ
unittests	`77.80% <97.22%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
runtime/ast/transaction_declaration.go	`78.94% <ø> (ø)`
runtime/interpreter/errors.go	`59.92% <ø> (ø)`
runtime/sema/resources.go	`88.44% <ø> (ø)`
types.go	`92.84% <ø> (ø)`
runtime/sema/checker.go	`92.11% <93.75%> (-0.03%)`	⬇️
runtime/sema/check_composite_declaration.go	`95.61% <100.00%> (+0.01%)`	⬆️
runtime/sema/check_member_expression.go	`97.81% <100.00%> (+0.03%)`	⬆️
runtime/sema/check_transaction_declaration.go	`96.02% <100.00%> (+0.22%)`	⬆️
runtime/sema/errors.go	`94.94% <100.00%> (+0.01%)`	⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2023-01-30T23:21:17Z

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/extended-transaction-format commit dc837ee
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

turbolent added 3 commits January 19, 2023 14:41

simplify resource invalidation recording

7abf054

refactor resource field invalidation check to function

48247ab

add support for resource-kinded fields in transaction roles: allow/re…

4cea08d

…quire invalidation of roles' members

turbolent added the Feature label Jan 19, 2023

turbolent requested a review from dreamsmasher January 19, 2023 23:57

turbolent requested a review from SupunS as a code owner January 19, 2023 23:57

turbolent self-assigned this Jan 19, 2023

turbolent requested a review from dsainati1 as a code owner January 19, 2023 23:57

turbolent commented Jan 19, 2023

View reviewed changes

turbolent commented Jan 20, 2023

View reviewed changes

turbolent mentioned this pull request Jan 20, 2023

FLIP - Extended Transaction Format onflow/flips#41

Closed

dsainati1 approved these changes Jan 20, 2023

View reviewed changes

turbolent added 2 commits January 20, 2023 15:06

improve identifier naming

2d73b12

check role resource field after invalidation

c566da8

dreamsmasher reviewed Jan 24, 2023

View reviewed changes

runtime/sema/checker.go Outdated Show resolved Hide resolved

dreamsmasher approved these changes Jan 24, 2023

View reviewed changes

improve field alignment

8a95517

turbolent force-pushed the bastian/extended-transaction-format-4 branch from 2b6f834 to 8a95517 Compare January 24, 2023 21:43

turbolent mentioned this pull request Jan 24, 2023

Forbid taking references of transaction roles #2268

Merged

6 tasks

SupunS approved these changes Jan 25, 2023

View reviewed changes

runtime/tests/checker/transactions_test.go Outdated Show resolved Hide resolved

turbolent added 4 commits January 27, 2023 11:59

improve message of invalid nested resource move error: include identi…

1a90380

…fier of member expression

refactor tests

7fb3e0a

test nested resource invalidation for transaction role resource-kinde…

a91d406

…d field

move checker config setup out of checking function

9fa982e

turbolent requested a review from SupunS January 27, 2023 20:23

SupunS approved these changes Jan 27, 2023

View reviewed changes

Base automatically changed from bastian/extended-transaction-format-3 to feature/extended-transaction-format January 30, 2023 22:37

turbolent added 2 commits January 30, 2023 14:52

Merge branch 'feature/extended-transaction-format' into bastian/exten…

21ee569

…ded-transaction-format-4

fix merge

27de6f2

roles are not stored in transaction type anymore, iterate over members

7d32929

turbolent requested a review from dsainati1 January 30, 2023 23:06

turbolent merged commit fa4cf3d into feature/extended-transaction-format Jan 31, 2023

turbolent deleted the bastian/extended-transaction-format-4 branch January 31, 2023 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for resource-kinded fields in transaction roles #2262

Add support for resource-kinded fields in transaction roles #2262

turbolent commented Jan 19, 2023 •

edited

Loading

turbolent Jan 19, 2023

turbolent Jan 20, 2023 •

edited

Loading

turbolent commented Jan 27, 2023

codecov bot commented Jan 30, 2023

github-actions bot commented Jan 30, 2023

		@@ -1366,32 +1366,13 @@ func (checker *Checker) recordResourceInvalidation(
		)
		}

Add support for resource-kinded fields in transaction roles #2262

Add support for resource-kinded fields in transaction roles #2262

Conversation

turbolent commented Jan 19, 2023 • edited Loading

Description

turbolent Jan 19, 2023

Choose a reason for hiding this comment

turbolent Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

turbolent commented Jan 27, 2023

codecov bot commented Jan 30, 2023

Codecov Report

github-actions bot commented Jan 30, 2023

Cadence Benchstat comparison

turbolent commented Jan 19, 2023 •

edited

Loading

turbolent Jan 20, 2023 •

edited

Loading