Skip to content

Conversation

@algobarb
Copy link
Contributor

@algobarb algobarb commented Jan 6, 2022

Summary

For nightly builds ("rel/nightly"), we want to have deadlock enabled.
For rel/beta and rel/stable, we want to make sure we can build and upload a binary with deadlock disabled so that it can be used for release testing and validation purposes.

Test Plan

Tested: upload_nightly_binaries darwin binary has deadlock enabled when running the uploaded binary.
Tested: upload_binaries binary has deadlock disabled when locally running the uploaded binary.

@algobarb algobarb reopened this Jan 6, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2022

Codecov Report

Merging #3381 (cd69d93) into master (cb1650e) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3381      +/-   ##
==========================================
+ Coverage   47.63%   47.65%   +0.01%     
==========================================
  Files         369      369              
  Lines       59759    59759              
==========================================
+ Hits        28468    28478      +10     
+ Misses      27973    27966       -7     
+ Partials     3318     3315       -3     
Impacted Files Coverage Δ
ledger/blockqueue.go 82.18% <0.00%> (-2.88%) ⬇️
catchup/service.go 69.40% <0.00%> (-0.75%) ⬇️
ledger/acctupdates.go 65.01% <0.00%> (+0.19%) ⬆️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
crypto/merkletrie/trie.go 68.61% <0.00%> (+2.18%) ⬆️
network/wsPeer.go 68.61% <0.00%> (+2.77%) ⬆️

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 cb1650e...cd69d93. Read the comment docs.

egieseke
egieseke previously approved these changes Jan 10, 2022
Copy link
Contributor

@egieseke egieseke left a comment

Choose a reason for hiding this comment

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

Looks good.

algojack
algojack previously approved these changes Jan 10, 2022
egieseke
egieseke previously approved these changes Jan 10, 2022
Copy link
Contributor

@egieseke egieseke left a comment

Choose a reason for hiding this comment

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

Looks good.

@algobarb algobarb dismissed stale reviews from egieseke and algojack via acadf4b January 11, 2022 14:38
Copy link
Contributor

@egieseke egieseke left a comment

Choose a reason for hiding this comment

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

Looks good!

@algobarb
Copy link
Contributor Author

@onetechnical Good to merge?

@algojohnlee algojohnlee merged commit e19fafb into master Jan 12, 2022
@algojohnlee algojohnlee deleted the circleci/1809 branch January 12, 2022 17:58
jannotti pushed a commit that referenced this pull request Jan 13, 2022
* testing: Fix unit test TestAsyncTelemetryHook_QueueDepth (#2685)

Fix the unit test TestAsyncTelemetryHook_QueueDepth

* Deprecate `FastPartitionRecovery` from `ConsensusParams` (#3386)

## Summary

This PR removes `FastPartitionRecovery` option from consensus parameters. The code now acts as if this value is set to true.

Closes https://github.com/algorand/go-algorand-internal/issues/1830.

## Test Plan

None.

* Remaking a PR for CI (#3398)

* Allow setting manager, reserve, freeze, and clawback at goal asset create

* Add e2e tests

* Add more tests for goal asset create flags

Co-authored-by: Fionna <[email protected]>

* [Other] CircleCI pipeline change for binary uploads (#3381)

For nightly builds ("rel/nightly"), we want to have deadlock enabled.

For rel/beta and rel/stable, we want to make sure we can build and upload a binary with deadlock disabled so that it can be used for release testing and validation purposes.

* signer.KeyDilution need not depend on config package (#3265)

crypto package need not depend on config.
There is an unnecessary dependency on config.

signer.KeyDilution takes the `config.ConsensusParams` as argument to pick the DefaultKeyDilution from it. 
This introduces dependency from the crypto package to config package. 
Instead, only the DefaultKeyDilution value can be passed to signer.KeyDilution.

* algodump is a tcpdump-like tool for algod's network protocol (#3166)

This PR introduces algodump, a tcpdump-like tool for monitoring algod network messages.

* Removing C/crypto dependencies from `data/abi` package (#3375)

* Feature Networks pipeline related changes (#3393)

Added support for not having certain files in signing script

Co-authored-by: Tsachi Herman <[email protected]>
Co-authored-by: Tolik Zinovyev <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Fionna <[email protected]>
Co-authored-by: algobarb <[email protected]>
Co-authored-by: Shant Karakashian <[email protected]>
Co-authored-by: Nickolai Zeldovich <[email protected]>
Co-authored-by: Hang Su <[email protected]>
jannotti added a commit that referenced this pull request Jan 18, 2022
* Three new globals for to help contract-to-contract usability

* detritis

* Check error

* doc comments

* Impose limits on the entire "tree" of inner calls.

This also increases the realism of testing of multiple app calls in a
group by creating the EvalParams with the real constructor, thus
getting the pooling stuff tested here without playing games
manipulating the ep after construction.

* Move appID tracking into EvalContext, out of LedgerForLogic

This change increases the seperation between AVM execution and the
ledger being used to lookup resources.  Previously, the ledger kept
track of the appID being executed, to offer a narrower interface to
those resources. But now, with app-to-app calls, the appID being
executed must change, and the AVM needs to maintain the current appID.

* Stupid linter

* Fix unit tests error messages

* Allow access to resources created in the same transaction group

The method will be reworked, but the tests are correct and want to get
them visible to team.

* Access to apps created in group

Also adds some tests that are currently skipped for testing
- access to addresses of newly created apps
- use of gaid in inner transactions

Both require some work to implement the thing being tested.

* Remove tracked created mechanism in favor of examining applydata.

* Allow v6 AVM code to use in-group created asas, apps (& their accts)

One exception - apps can not mutate (put or del) keys from the app
accounts, because EvalDelta cannot encode such changes.

* lint docs

* typo

* The review dog needs obedience training.

* Use one EvalParams for logic evals, another for apps in dry run

We used to use one ep per transaction, shared between sig and and
app. But the new model of ep usage is to keep using one while
evaluating an entire group.

The app ep is now built logic.NewAppEvalParams which, hopefully, will
prevent some bugs when we change something in the EvalParams and don't
reflect it in what was a "raw" EvalParams construction in debugger and
dry run.

* Use logic.NewAppEvalParams to decrease copying and bugs in debugger

* Simplify use of NewEvalParams. No more nil return when no apps.

This way, NewEvalParams can be used for all creations of EvalParams,
whether they are intended for logicsig or app use, greatly simplifying
the way we make them for use by dry run or debugger (where they serve
double duty).

* Remove explicit PastSideEffects handling in tealdbg

* Always create EvalParams to evaluate a transaction group.

We used to have an optimization to avoid creating EvalParams unless
there was an app call in the transaction group.  But the interface to
allow transaction processing to communicate changes into the
EvalParams is complicated by that (we must only do it if there is
one!)

This also allows us to use the same construction function for eps
created for app and logic evaluation, simplifying dry-run and
debugger.

The optimization is less needed now anyway:
1) The ep is now shared for the whole group, so it's only one.
2) The ep is smaller now, as we only store nil pointers instead of
larger scratch space objects for non-app calls.

* Correct mistaken commit

* Spec improvments

* More spec improvments, including resource "availability"

* Recursively return inner transaction tree

* Lint

* No need for ConfirmedRound, so don't deref a nil pointer!

* license check

* Shut up, dawg.

* testing: Fix unit test TestAsyncTelemetryHook_QueueDepth (#2685)

Fix the unit test TestAsyncTelemetryHook_QueueDepth

* Deprecate `FastPartitionRecovery` from `ConsensusParams` (#3386)

## Summary

This PR removes `FastPartitionRecovery` option from consensus parameters. The code now acts as if this value is set to true.

Closes algorand/go-algorand-internal#1830.

## Test Plan

None.

* base64 merge cleanup

* Remaking a PR for CI (#3398)

* Allow setting manager, reserve, freeze, and clawback at goal asset create

* Add e2e tests

* Add more tests for goal asset create flags

Co-authored-by: Fionna <[email protected]>

* Remove the extraneous field type arrays.

* bsqrt

* acct_holding_get, a unified opcode for account field access

* Thanks, dawg

* [Other] CircleCI pipeline change for binary uploads (#3381)

For nightly builds ("rel/nightly"), we want to have deadlock enabled.

For rel/beta and rel/stable, we want to make sure we can build and upload a binary with deadlock disabled so that it can be used for release testing and validation purposes.

* signer.KeyDilution need not depend on config package (#3265)

crypto package need not depend on config.
There is an unnecessary dependency on config.

signer.KeyDilution takes the `config.ConsensusParams` as argument to pick the DefaultKeyDilution from it. 
This introduces dependency from the crypto package to config package. 
Instead, only the DefaultKeyDilution value can be passed to signer.KeyDilution.

* CR and more spec simplification

* algodump is a tcpdump-like tool for algod's network protocol (#3166)

This PR introduces algodump, a tcpdump-like tool for monitoring algod network messages.

* Removing C/crypto dependencies from `data/abi` package (#3375)

* Feature Networks pipeline related changes (#3393)

Added support for not having certain files in signing script

* e2e test for inner transaction appls

* testing: Add slightly more coverage to TestAcctUpdatesLookupRetry (#3384)

Add slightly more coverage to TestAcctUpdatesLookupRetry

* add context to (most) agreement logged writes (#3411)

Current agreement code only writes a `context : agreement` to a subset of the logged messages.
This change extends the said entry, which would make it easier to pre-process logs entries by their corresponding component. The change in this PR is focused on:
1. make sure that the "root" agreement logger always injects the `context : agreement` argument.
2. change the various locations in the agreement code to use the root agreement logger instead of referring to the application-global instance (`logging.Base()`).

* network: faster node shutdown (#3416)

During the node shutdown, all the current outgoing connections are being disconnected.
Since these connections are web sockets, they require a close connection message to be sent.
However, sending this message can take a while, and in situations where the other party has already shut down, we might never get a response. That, in turn, would lead the node waiting until the deadline is reached.

The current deadline was 5 seconds. This PR changes the deadline during shutdown to be 50ms.

* Give max group size * 16 inner txns, regardless of apps present

* Adjust test for allowing 256 inners

Co-authored-by: Tsachi Herman <[email protected]>
Co-authored-by: Tolik Zinovyev <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Fionna <[email protected]>
Co-authored-by: algobarb <[email protected]>
Co-authored-by: Shant Karakashian <[email protected]>
Co-authored-by: Nickolai Zeldovich <[email protected]>
Co-authored-by: Hang Su <[email protected]>
Co-authored-by: chris erway <[email protected]>
@algobarb algobarb mentioned this pull request Jan 18, 2022
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