Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable account linking / AuthAccount capabilities on all networks except Mainnet #3910

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

turbolent
Copy link
Member

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

FVM Benchstat comparison

This branch with compared with the base branch onflow:v0.29 commit 1d1a44d

The command (for i in {1..7}; do go test ./fvm ./engine/execution/computation --bench . --tags relic -shuffle=on --benchmem --run ^$; done) was used.

Collapsed results for better readability

newCustomRuntime func() runtime.Runtime,
) ReusableCadenceRuntimePool {
var pool chan *ReusableCadenceRuntime
if poolSize > 0 {
pool = make(chan *ReusableCadenceRuntime, poolSize)
}

// Enable account linking on all networks except Mainnet
config.AccountLinkingEnabled = chainID != flow.Mainnet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: I would prefer this set being higher up so chainID doesn't need to get sent so deep just for this.
But since this is just temporary for 0.29 its ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 if you're porting this to head

Copy link
Member Author

@turbolent turbolent Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure how to enforce this setting consistently everywhere. It would be great if someone from the execution team with more knowledge about flow-go/FVM could advise / perform the refactor.

This is not just temporary for 0.29, but will also get ported to master.

newCustomRuntime func() runtime.Runtime,
) ReusableCadenceRuntimePool {
var pool chan *ReusableCadenceRuntime
if poolSize > 0 {
pool = make(chan *ReusableCadenceRuntime, poolSize)
}

// Enable account linking on all networks except Mainnet
config.AccountLinkingEnabled = chainID != flow.Mainnet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 if you're porting this to head

@codecov-commenter
Copy link

Codecov Report

Merging #3910 (1e55c82) into v0.29 (47be83a) will decrease coverage by 1.64%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##            v0.29    #3910      +/-   ##
==========================================
- Coverage   53.47%   51.84%   -1.64%     
==========================================
  Files         812      178     -634     
  Lines       76105    16672   -59433     
==========================================
- Hits        40695     8643   -32052     
+ Misses      32113     7504   -24609     
+ Partials     3297      525    -2772     
Flag Coverage Δ
unittests 51.84% <83.33%> (-1.64%) ⬇️

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

Impacted Files Coverage Δ
fvm/runtime/reusable_cadence_runtime.go 43.69% <72.72%> (+2.78%) ⬆️
fvm/environment/env.go 100.00% <100.00%> (ø)
fvm/environment/runtime.go 100.00% <100.00%> (ø)
...s/hotstuff/timeoutaggregator/timeout_collectors.go 76.11% <0.00%> (-5.98%) ⬇️
consensus/hotstuff/eventloop/event_loop.go 75.11% <0.00%> (-4.23%) ⬇️
fvm/transactionInvoker.go 75.96% <0.00%> (-0.97%) ⬇️
engine/execution/computation/manager.go
cmd/bootstrap/run/cluster_qc.go
network/proxy/conduit.go
engine/execution/rpc/engine.go
... and 630 more

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

@turbolent
Copy link
Member Author

bors merge

bors bot added a commit that referenced this pull request Feb 9, 2023
3910: Enable account linking / AuthAccount capabilities on all networks except Mainnet r=turbolent a=turbolent



Co-authored-by: Bastian Müller <bastian@axiomzen.co>
@bors
Copy link
Contributor

bors bot commented Feb 9, 2023

@turbolent
Copy link
Member Author

bors merge

bors bot added a commit that referenced this pull request Feb 9, 2023
3910: Enable account linking / AuthAccount capabilities on all networks except Mainnet r=turbolent a=turbolent



Co-authored-by: Bastian Müller <bastian@axiomzen.co>
@bors
Copy link
Contributor

bors bot commented Feb 9, 2023

Build failed:

@turbolent
Copy link
Member Author

bors merge

bors bot added a commit that referenced this pull request Feb 9, 2023
3910: Enable account linking / AuthAccount capabilities on all networks except Mainnet r=turbolent a=turbolent



Co-authored-by: Bastian Müller <bastian@axiomzen.co>
@bors
Copy link
Contributor

bors bot commented Feb 9, 2023

Canceled.

@turbolent
Copy link
Member Author

bors merge

@bors bors bot merged commit e7dd12e into v0.29 Feb 9, 2023
@bors bors bot deleted the bastian/enable-authaccount-capabilities branch February 9, 2023 19:25
bors bot added a commit that referenced this pull request Mar 10, 2023
4017: Update to Cadence v0.36.0 r=turbolent a=dsainati1



## Description

Update to:
- [onflow/flow-go-sdk v0.35.0](https://github.com/onflow/flow-go-sdk/releases/tag/v0.35.0)
- [onflow/cadence v0.36.0](https://github.com/onflow/cadence/releases/tag/v0.36.0)

Port from v0.29:
- #3910
- #3920
- #3969
- #3997

Co-authored-by: Daniel Sainati <sainatidaniel@gmail.com>
Co-authored-by: Bastian Müller <bastian@axiomzen.co>
Co-authored-by: Supun Setunga <supun.setunga@gmail.com>
Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants