fix!: Merklerootmnlist calc bls scheme #5157
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PastaPastaPasta
requested changes
Jan 12, 2023
PastaPastaPasta
approved these changes
Jan 12, 2023
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Nov 20, 2023
fadfd84 test: Remove unused connect_nodes_bi (MarcoFalke) fa3b9ee scripted-diff: test: Replace connect_nodes_bi with connect_nodes (MarcoFalke) faaee1e test: Use connect_nodes when connecting nodes in the test_framework (MarcoFalke) 1111bb9 test: Reformat python imports to aid scripted diff (MarcoFalke) Pull request description: By default all test nodes are connected in a chain. However, instead of just a single connection between each pair of nodes, we end up with up to four connections for a "middle" node (two outbound, two inbound, from each side). This is generally redundant (tx and block relay should succeed with just a single connection) and confusing. For example, test timeouts after a call to `sync_` may be racy and hard to reproduce. On top of that, the test `debug.log`s are hard to read because txs and block invs may be relayed on the same connection multiple times. Fix this by inlining `connect_nodes_bi` in the two tests that need it, and then replace it with a single `connect_nodes` in all other tests. Historic background: `connect_nodes_bi` has been introduced as a (temporary?) workaround for bug dashpay#5113 and dashpay#5138, which has long been fixed in dashpay#5157 and dashpay#5662. ACKs for top commit: laanwj: ACK fadfd84 jonasschnelli: utACK fadfd84 - more of less a cleanup PR. promag: Tested ACK fadfd84, ran extended tests. Tree-SHA512: 2d027a8fd150749c071b64438a0a78ec922178628a7dbb89fd1212b0fa34febd451798c940101155d3617c0426c2c4865174147709894f1f1bb6cfa336aa7e24
gades
pushed a commit
to piratecash/pirate
that referenced
this pull request
Dec 9, 2023
fadfd84 test: Remove unused connect_nodes_bi (MarcoFalke) fa3b9ee scripted-diff: test: Replace connect_nodes_bi with connect_nodes (MarcoFalke) faaee1e test: Use connect_nodes when connecting nodes in the test_framework (MarcoFalke) 1111bb9 test: Reformat python imports to aid scripted diff (MarcoFalke) Pull request description: By default all test nodes are connected in a chain. However, instead of just a single connection between each pair of nodes, we end up with up to four connections for a "middle" node (two outbound, two inbound, from each side). This is generally redundant (tx and block relay should succeed with just a single connection) and confusing. For example, test timeouts after a call to `sync_` may be racy and hard to reproduce. On top of that, the test `debug.log`s are hard to read because txs and block invs may be relayed on the same connection multiple times. Fix this by inlining `connect_nodes_bi` in the two tests that need it, and then replace it with a single `connect_nodes` in all other tests. Historic background: `connect_nodes_bi` has been introduced as a (temporary?) workaround for bug dashpay#5113 and dashpay#5138, which has long been fixed in dashpay#5157 and dashpay#5662. ACKs for top commit: laanwj: ACK fadfd84 jonasschnelli: utACK fadfd84 - more of less a cleanup PR. promag: Tested ACK fadfd84, ran extended tests. Tree-SHA512: 2d027a8fd150749c071b64438a0a78ec922178628a7dbb89fd1212b0fa34febd451798c940101155d3617c0426c2c4865174147709894f1f1bb6cfa336aa7e24
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Code was still using legacy scheme for
pubKeyOperatorof eachCSimplifiedMNListEntrywhile calculatingMerklerootMNListwhile v19 HF was active.What was done?
When building
CSimplifiedMNListforMerklerootMNList, this PR adds the correct set ofnVersionforCSimplifiedMNListEntryso that basic scheme will be used instead of legacy.Furthermore, DIP3 unit tests suites for v19 will test for 1000 blocks instead of 900 to cover v19 activated period.
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only