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

Update to stable Cadence #4567

Merged
merged 39 commits into from
Aug 29, 2023
Merged

Update to stable Cadence #4567

merged 39 commits into from
Aug 29, 2023

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Jul 18, 2023

Feature branch for stable cadence updates to integrate the new token standards, core contracts, and other features of stable cadence

Depends on #4655

@github-actions
Copy link
Contributor

github-actions bot commented Jul 19, 2023

FVM Benchstat comparison

This branch with compared with the base branch onflow:feature/stable-cadence commit cc67154

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

@SupunS SupunS self-assigned this Jul 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

Merging #4567 (d8230fb) into feature/stable-cadence (c422431) will increase coverage by 0.08%.
The diff coverage is 79.48%.

@@                    Coverage Diff                     @@
##           feature/stable-cadence    #4567      +/-   ##
==========================================================
+ Coverage                   54.66%   54.75%   +0.08%     
==========================================================
  Files                         917      917              
  Lines                       85633    85537      -96     
==========================================================
+ Hits                        46808    46832      +24     
+ Misses                      35231    35113     -118     
+ Partials                     3594     3592       -2     
Flag Coverage Δ
unittests 54.75% <79.48%> (+0.08%) ⬆️

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

Files Changed Coverage Δ
cmd/util/ledger/reporters/account_reporter.go 0.00% <0.00%> (ø)
fvm/environment/account_key_updater.go 23.50% <ø> (+4.01%) ⬆️
fvm/environment/meter.go 77.27% <ø> (ø)
fvm/meter/memory_meter.go 88.88% <ø> (ø)
utils/unittest/execution_state.go 31.81% <0.00%> (ø)
utils/unittest/fixtures.go 0.00% <0.00%> (ø)
fvm/bootstrap.go 88.54% <96.87%> (+0.14%) ⬆️

... and 14 files with indirect coverage changes

@SupunS SupunS closed this Aug 24, 2023
@SupunS SupunS reopened this Aug 24, 2023
@SupunS
Copy link
Member Author

SupunS commented Aug 24, 2023

There are some tests failing that seem to be unrelated to the cadence change. Can someone with more context have a look please?

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -10,7 +10,7 @@ import (
const (
getInfoForProposedNodesScript = `
import FlowIDTableStaking from 0x%s
pub fun main(): [FlowIDTableStaking.NodeInfo] {
access(all) fun main(): [FlowIDTableStaking.NodeInfo] {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add line breaks after access modifiers, as they are now quite long, making the function signature hard to read

Copy link
Member

Choose a reason for hiding this comment

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

idk, I feel like that makes the code looks pretty awkward

Copy link
Member Author

Choose a reason for hiding this comment

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

I will update the tests where necessary in a follow-up PR.

@@ -1,7 +1,7 @@
import Crypto
import FungibleToken from 0xFUNGIBLETOKENADDRESS
import FlowToken from 0xFLOWTOKENADDRESS
import FlowIDTableStaking from 0xIDENTITYTABLEADDRESS
import FlowIDTableStaking from "FlowIDTableStaking"
Copy link
Member

Choose a reason for hiding this comment

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

Can the other 0xFOO "addresses" in the other imports also be replaced to the string literal?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I'm working on it. I won't be able to get to it in this PR, but maybe in the next one

@SupunS
Copy link
Member Author

SupunS commented Aug 25, 2023

There is still two one test failing:

  • TestPassThrough/TestSealingAndVerificationPassThrough in Integration Tests (make -C integration bft-tests)
  • TestFullGossipSubConnectivityAmongHonestNodesWithMaliciousMajority in Unit Tests (network)

Don't have much clue on why they are failing / how to fix.

@SupunS
Copy link
Member Author

SupunS commented Aug 28, 2023

bors merge

bors bot added a commit that referenced this pull request Aug 28, 2023
4567: Update to Cadence v0.39.13-stable-cadence r=SupunS a=SupunS

Feature branch for stable cadence updates to integrate the new token standards, core contracts, and other features of stable cadence

Depends on #4655

Co-authored-by: Yahya <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Yahya Hassanzadeh, Ph.D <yhassanzadeh@ieee.org>
Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Yahya Hassanzadeh, Ph.D <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Supun Setunga <supun.setunga@gmail.com>
Co-authored-by: Misha <misha@gomisha.com>
Co-authored-by: Kay-Zee <kan@axiomzen.co>
Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com>
Co-authored-by: Kan Zhang <kan@axiomzen.co>
Co-authored-by: Misha <misha.rybalov@dapperlabs.com>
@SupunS
Copy link
Member Author

SupunS commented Aug 28, 2023

bors cancel

@bors
Copy link
Contributor

bors bot commented Aug 28, 2023

Canceled.

@SupunS SupunS changed the title Update to Cadence v0.39.13-stable-cadence Update to stable Cadence Aug 29, 2023
@SupunS SupunS merged commit a08b365 into feature/stable-cadence Aug 29, 2023
36 of 37 checks passed
@SupunS SupunS deleted the supun/stable-cadence branch August 29, 2023 18:12
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants