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

DevMode: create a block for external txns only #3784

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

algonautshant
Copy link
Contributor

In DevMode, do not create a block for internal events (i.e. compact-cert creation).
This is to assure reproducibility and eliminate the random shifts in round numbers.

tsachiherman
tsachiherman previously approved these changes Mar 17, 2022
tsachiherman
tsachiherman previously approved these changes Mar 17, 2022
Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

I understand the fix but I do not really like BroadcastCompactCertSignedTxGroup function that bypasses devmode for compact cert.
Maybe add some code into BroadcastSignedTxGroup for txn group analysis and bypass devnode for the internal transactions like compact cert?

node/node.go Outdated
return node.broadcastSignedTxGroup(txgroup)
}

// BroadcastCompactCertSignedTxGroup broadcasts a transaction group that has already been signed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain the difference with BroadcastSignedTxGroup in a comment?

@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2022

Codecov Report

Merging #3784 (02c4ec1) into master (af3b5d3) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #3784      +/-   ##
==========================================
- Coverage   49.88%   49.85%   -0.03%     
==========================================
  Files         392      392              
  Lines       68685    68687       +2     
==========================================
- Hits        34264    34247      -17     
- Misses      30669    30685      +16     
- Partials     3752     3755       +3     
Impacted Files Coverage Δ
node/node.go 23.26% <0.00%> (-0.08%) ⬇️
compactcert/builder.go 65.57% <100.00%> (ø)
ledger/blockqueue.go 81.03% <0.00%> (-4.03%) ⬇️
ledger/tracker.go 74.67% <0.00%> (-2.15%) ⬇️
cmd/tealdbg/debugger.go 71.42% <0.00%> (-0.99%) ⬇️
ledger/acctupdates.go 68.51% <0.00%> (-0.93%) ⬇️
catchup/service.go 69.38% <0.00%> (-0.75%) ⬇️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
crypto/merkletrie/trie.go 68.61% <0.00%> (+2.18%) ⬆️

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 af3b5d3...02c4ec1. Read the comment docs.

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.

4 participants