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.

Sign up for GitHub

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

Jump to bottom

Fix GetSigOpCount and revert earlier workaround #493

Merged
merged 3 commits into from
Apr 6, 2021
Merged

Fix GetSigOpCount and revert earlier workaround #493

merged 3 commits into from
Apr 6, 2021

Conversation

quantumagi
Copy link
Contributor

No description provided.

@quantumagi quantumagi requested a review from zeptin March 31, 2021 06:34
@noescape00
Copy link
Contributor

Looks good. Rerun CI please

zeptin
zeptin reviewed Mar 31, 2021
View reviewed changes
src/NBitcoin/Script.cs
@@ -865,7 +865,9 @@ public uint GetSigOpCount(bool fAccurate)
n++;
else if (op.Code == OpcodeType.OP_CHECKMULTISIG || op.Code == OpcodeType.OP_CHECKMULTISIGVERIFY)
{
if (fAccurate && lastOpcode != null && lastOpcode.Code >= OpcodeType.OP_1 && lastOpcode.Code <= OpcodeType.OP_16)
if (fAccurate && network?.Federations != null && lastOpcode.Code == OpcodeType.OP_FEDERATION)
n += (uint)network.Federations.GetOnlyFederation().GetFederationDetails().transactionSigningKeys.Length;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose that, strictly speaking, this uses how many members there are in the federation in total rather than the number that actually signed a given transaction. So the computed sigOp count will land up somewhat higher than it could be.

Copy link
Contributor Author

@quantumagi quantumagi Apr 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I assume that would be consistent with the existing behaviour for non-federation multi sig below which also does not use the actual number of signatures - only the number accepted by the script. E.g. looking at:
s1 = s1 + OpcodeType.OP_1 + dummy.ToBytes() + dummy.ToBytes() + OpcodeType.OP_2 + OpcodeType.OP_CHECKMULTISIG;
The number just preceding the OP_CHECKMULTISIG would be the max number of signatures that CAN be passed.

zeptin
zeptin approved these changes Mar 31, 2021
View reviewed changes
Copy link
Collaborator

@zeptin zeptin left a comment

Choose a reason for hiding this comment

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

A test sync of the StraxMain and CirrusMain networks should be performed

@quantumagi quantumagi merged commit a45758e into stratisproject:master Apr 6, 2021
zeptin added a commit that referenced this pull request Apr 6, 2021
@zeptin
Dynamic fee (#505)
5f232b3 
* Dynamic fee computation

* Dynamic fee

* Interflux  (#481)

* Update NBitcoin.csproj (#391)

* Interflux (#397)

* Initial InterFlux version

* Refactor

* Modify DI sequence (#398)

* Modify DI sequence

* Update NEthereum and Stratis.Patricia (#412)

* Initial InterFlux version

* Update NBitcoin.csproj (#391)

* Pass along height information for OP_FEDERATION (#379)

* Pass along height information for OP_FEDERATION

* Add defaults

* Add defaults

* Filter Transaction History by Transaction Id (#393)

* Update WalletService.cs

* Add Filter

* Fix Test

* Perform check for cold staking txs

* Refactor

* Modify DI sequence

* Add mainnet check points (#400)

* Add MainNet check points

* Fix Tests

* Change gas limit upper bound to 250,000 (#399)

* Add UInt256, serialization methods and CLR updates (#365)

* Add UInt256 serialization methods

* Refactor

* Use UInt256 in InternalExecutor

* Undo unrelated change

* Update TestMessage

* Revert type updates to InternalExecutor

* Update Serializer and PersistentState

* Update package versions

* Update SC packages to version 1.4.0-alpha

* Add serializers and enums

* Add primitive serializer

* Update StandardToken.cs

* Revert change

* Update integration tests

* Use switch/case in Serialize/Deserialize

* Update package versions and remove workaround

* Ensure the right Stratis.SmartContracts.Standards is used by contracts

* Changes based on feedback

* Fix primitive deserialize

* Update standards dll name

* Fix legacy standards dll path

* Don't upgrade standards package of node

* Don't upgrade StandardToken of node

* Don't upgrade standards package of node

* Add Standards forwards compatibility

* Add test case for futuristic contracts

* Fix Token_Standards_Test

* Update RPC_GetReceipt_Returns_Value test

* Cleanup non-required changes

* Validate standards assembly

* Disable automatic downloads

* Add LegacyStandardsDLLs

* Update test

* Bump package versions for Core and Networks

* Bump observer version

* Update package versions

* Refactor

* Revert gitignore

* Revert unnecessary changes

* Revert unnecessary changes

* Update Stratis.SmartContracts.xxx to 2.0.0.0

* Update package version for Stratis.SmartContracts.Tests.Common

* Add TestNet checkpoints (#402)

* Update StraxTest.cs

* Add testnet checkpoints

* Fix Tests

* Update Nuget Scripts and Collateral/FederatedPeg versions

* Bump Version (#403)

* Update some test projects PackageIds and versions

* Update PushNuget.ps1

* Update MaxStandardSigOpsCost and CCTS Max Partials Txs (#404)

* Update Stratis.SmartContracts.CLR.Validation's allowed primitive types (#405)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions (#406)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions

* Bump Stratis.Bicoin.Features.SmartContracts as well

* Bump Stratis.SmartContracts.Networks

* Update PushSCNuget.ps1

* Fix Finality Bug (#407)

* Fix Finalized Bug

* Fix Build

* Trigger CI

* Fix Tests

* Comments

* Console Cleanup (#408)

* Suppress bench logs

* Done

* Update NodeStats.cs

* Fix Test

* Bump version prior to fixing NuGet dependencies for  Stratis.SmartContracts.Tests.Common (#409)

* Rocks DB (#401)

* WIP

* Fix DB connections

* Update CirrusD

* Add RocksDbNative

* Update Stratis.CirrusD.csproj

* Add Snappy

* Fix BlockStoreQueue

* Add switch to program.cs

* Fix Logger

* Cleanup Console Logs

* Add RocksDbChainStore

* Update BlockPuller.cs

* Implement RocksDb KeyValueStore

* Add dbtype switch to StraxD

* NodeStats changes

* KeyValue fix

* Fix Build

* Voting / Wallet Log

* Update ConnectionManager.cs

* Hide bench stats

* Update VersionProvider.cs

* Self Review

* Fix Test

* Update ConnectionManager.cs

* Fix Locks in StakeDB

* Create RocksDbProvenBlockHeaderRepository

* Update RocksDbProvenBlockHeaderRepository.cs

* Add network type to console

* Fix Test

* Update NodeStats.cs

* Add datafolder to nodestats

* Fix Merge

* Revert

* Revert launchsettings

* Add ColdStaking to PushNuGet script (#410)

* Add ColdStaking to PushNuGet script

* Bump Stratis.Bitcoin.IntegrationTests.Common to 1.0.7

* Bump Stratis.Core.Tests.Common to 1.0.7

* Update NEthereum and Stratis.Patricia

* SLN changes

* Clean up commits

* Remove launchSettings change

Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com>
Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>

* Add references (#413)

* Initial InterFlux version

* Update NBitcoin.csproj (#391)

* Pass along height information for OP_FEDERATION (#379)

* Pass along height information for OP_FEDERATION

* Add defaults

* Add defaults

* Filter Transaction History by Transaction Id (#393)

* Update WalletService.cs

* Add Filter

* Fix Test

* Perform check for cold staking txs

* Refactor

* Modify DI sequence

* Add mainnet check points (#400)

* Add MainNet check points

* Fix Tests

* Change gas limit upper bound to 250,000 (#399)

* Add UInt256, serialization methods and CLR updates (#365)

* Add UInt256 serialization methods

* Refactor

* Use UInt256 in InternalExecutor

* Undo unrelated change

* Update TestMessage

* Revert type updates to InternalExecutor

* Update Serializer and PersistentState

* Update package versions

* Update SC packages to version 1.4.0-alpha

* Add serializers and enums

* Add primitive serializer

* Update StandardToken.cs

* Revert change

* Update integration tests

* Use switch/case in Serialize/Deserialize

* Update package versions and remove workaround

* Ensure the right Stratis.SmartContracts.Standards is used by contracts

* Changes based on feedback

* Fix primitive deserialize

* Update standards dll name

* Fix legacy standards dll path

* Don't upgrade standards package of node

* Don't upgrade StandardToken of node

* Don't upgrade standards package of node

* Add Standards forwards compatibility

* Add test case for futuristic contracts

* Fix Token_Standards_Test

* Update RPC_GetReceipt_Returns_Value test

* Cleanup non-required changes

* Validate standards assembly

* Disable automatic downloads

* Add LegacyStandardsDLLs

* Update test

* Bump package versions for Core and Networks

* Bump observer version

* Update package versions

* Refactor

* Revert gitignore

* Revert unnecessary changes

* Revert unnecessary changes

* Update Stratis.SmartContracts.xxx to 2.0.0.0

* Update package version for Stratis.SmartContracts.Tests.Common

* Add TestNet checkpoints (#402)

* Update StraxTest.cs

* Add testnet checkpoints

* Fix Tests

* Update Nuget Scripts and Collateral/FederatedPeg versions

* Bump Version (#403)

* Update some test projects PackageIds and versions

* Update PushNuget.ps1

* Update MaxStandardSigOpsCost and CCTS Max Partials Txs (#404)

* Update Stratis.SmartContracts.CLR.Validation's allowed primitive types (#405)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions (#406)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions

* Bump Stratis.Bicoin.Features.SmartContracts as well

* Bump Stratis.SmartContracts.Networks

* Update PushSCNuget.ps1

* Fix Finality Bug (#407)

* Fix Finalized Bug

* Fix Build

* Trigger CI

* Fix Tests

* Comments

* Console Cleanup (#408)

* Suppress bench logs

* Done

* Update NodeStats.cs

* Fix Test

* Bump version prior to fixing NuGet dependencies for  Stratis.SmartContracts.Tests.Common (#409)

* Rocks DB (#401)

* WIP

* Fix DB connections

* Update CirrusD

* Add RocksDbNative

* Update Stratis.CirrusD.csproj

* Add Snappy

* Fix BlockStoreQueue

* Add switch to program.cs

* Fix Logger

* Cleanup Console Logs

* Add RocksDbChainStore

* Update BlockPuller.cs

* Implement RocksDb KeyValueStore

* Add dbtype switch to StraxD

* NodeStats changes

* KeyValue fix

* Fix Build

* Voting / Wallet Log

* Update ConnectionManager.cs

* Hide bench stats

* Update VersionProvider.cs

* Self Review

* Fix Test

* Update ConnectionManager.cs

* Fix Locks in StakeDB

* Create RocksDbProvenBlockHeaderRepository

* Update RocksDbProvenBlockHeaderRepository.cs

* Add network type to console

* Fix Test

* Update NodeStats.cs

* Add datafolder to nodestats

* Fix Merge

* Revert

* Revert launchsettings

* Add ColdStaking to PushNuGet script (#410)

* Add ColdStaking to PushNuGet script

* Bump Stratis.Bitcoin.IntegrationTests.Common to 1.0.7

* Bump Stratis.Core.Tests.Common to 1.0.7

* Update NEthereum and Stratis.Patricia

* SLN changes

* Clean up commits

* Remove launchSettings change

* Add references

Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com>
Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>

* Conditional invocation (#419)

* Update LaunchSidechainMasternode.ps1 (#420)

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Move interflux namespace (#428)

* Fix null reference (#429)

* Update to 1.0.7.2 (#430)

* Update NBitcoin.csproj (#391)

* Pass along height information for OP_FEDERATION (#379)

* Pass along height information for OP_FEDERATION

* Add defaults

* Add defaults

* Filter Transaction History by Transaction Id (#393)

* Update WalletService.cs

* Add Filter

* Fix Test

* Perform check for cold staking txs

* Add mainnet check points (#400)

* Add MainNet check points

* Fix Tests

* Change gas limit upper bound to 250,000 (#399)

* Add UInt256, serialization methods and CLR updates (#365)

* Add UInt256 serialization methods

* Refactor

* Use UInt256 in InternalExecutor

* Undo unrelated change

* Update TestMessage

* Revert type updates to InternalExecutor

* Update Serializer and PersistentState

* Update package versions

* Update SC packages to version 1.4.0-alpha

* Add serializers and enums

* Add primitive serializer

* Update StandardToken.cs

* Revert change

* Update integration tests

* Use switch/case in Serialize/Deserialize

* Update package versions and remove workaround

* Ensure the right Stratis.SmartContracts.Standards is used by contracts

* Changes based on feedback

* Fix primitive deserialize

* Update standards dll name

* Fix legacy standards dll path

* Don't upgrade standards package of node

* Don't upgrade StandardToken of node

* Don't upgrade standards package of node

* Add Standards forwards compatibility

* Add test case for futuristic contracts

* Fix Token_Standards_Test

* Update RPC_GetReceipt_Returns_Value test

* Cleanup non-required changes

* Validate standards assembly

* Disable automatic downloads

* Add LegacyStandardsDLLs

* Update test

* Bump package versions for Core and Networks

* Bump observer version

* Update package versions

* Refactor

* Revert gitignore

* Revert unnecessary changes

* Revert unnecessary changes

* Update Stratis.SmartContracts.xxx to 2.0.0.0

* Update package version for Stratis.SmartContracts.Tests.Common

* Add TestNet checkpoints (#402)

* Update StraxTest.cs

* Add testnet checkpoints

* Fix Tests

* Update Nuget Scripts and Collateral/FederatedPeg versions

* Bump Version (#403)

* Update some test projects PackageIds and versions

* Update PushNuget.ps1

* Update MaxStandardSigOpsCost and CCTS Max Partials Txs (#404)

* Update Stratis.SmartContracts.CLR.Validation's allowed primitive types (#405)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions (#406)

* Update validator's primitives

* Bump package version

* Bump Stratis.SmartContracts.Core and Stratis.SmartContracts.CLR versions

* Bump Stratis.Bicoin.Features.SmartContracts as well

* Bump Stratis.SmartContracts.Networks

* Update PushSCNuget.ps1

* Fix Finality Bug (#407)

* Fix Finalized Bug

* Fix Build

* Trigger CI

* Fix Tests

* Comments

* Console Cleanup (#408)

* Suppress bench logs

* Done

* Update NodeStats.cs

* Fix Test

* Bump version prior to fixing NuGet dependencies for  Stratis.SmartContracts.Tests.Common (#409)

* Rocks DB (#401)

* WIP

* Fix DB connections

* Update CirrusD

* Add RocksDbNative

* Update Stratis.CirrusD.csproj

* Add Snappy

* Fix BlockStoreQueue

* Add switch to program.cs

* Fix Logger

* Cleanup Console Logs

* Add RocksDbChainStore

* Update BlockPuller.cs

* Implement RocksDb KeyValueStore

* Add dbtype switch to StraxD

* NodeStats changes

* KeyValue fix

* Fix Build

* Voting / Wallet Log

* Update ConnectionManager.cs

* Hide bench stats

* Update VersionProvider.cs

* Self Review

* Fix Test

* Update ConnectionManager.cs

* Fix Locks in StakeDB

* Create RocksDbProvenBlockHeaderRepository

* Update RocksDbProvenBlockHeaderRepository.cs

* Add network type to console

* Fix Test

* Update NodeStats.cs

* Add datafolder to nodestats

* Fix Merge

* Revert

* Revert launchsettings

* Add ColdStaking to PushNuGet script (#410)

* Add ColdStaking to PushNuGet script

* Bump Stratis.Bitcoin.IntegrationTests.Common to 1.0.7

* Bump Stratis.Core.Tests.Common to 1.0.7

* Update to Stratis.RocksDb (#415)

* Prevent changing published packages w/o bumping local version (#411)

* Prevent changing package code of published version - must bump version

* Update comments

* Small refactor

* Ensure Release folder exists

* Check version changes related to project references

* Bump versions

* Refactor

* Refactor

* Fix wallet feature vesion

* Update RocksDb dylibs (#417)

* Update to Stratis.RocksDb

* Update to version 0.0.9.1

* Update RocksDb Nugets

* Update Rocksdb (#418)

* Revert "Dont fetch/process deposits until the CCTS has cleared its suspended transfers (#367)"

This reverts commit 334b4f0.

* Update Nuget

* Update CirrusMain.cs (#422)

* Consolidate outstanding console changes for the MS/Miner (#423)

* update RocksDb Nuget (#424)

* Update Stratis.Bitcoin Assembly info to version 1.0.7.2

* Fix Seeder (#425)

* Fix AddressIndexer Console (#426)

* Update ConnectionManager.cs (#427)

* Fix merge conflicts

Co-authored-by: Francois de la Rouviere <fassadlr@gmail.com>
Co-authored-by: quantumagi <someguy.fromafrica@gmail.com>

* Add signature to coordination payload and improve logging (#432)

* Fix constructor (#433)

* Configurable gas limit and price (#434)

* Recreate event filter on RPC exception

* Clean up

* Fixes & updated deposit confirmation intervals

* Asyncify

* Remove unused

* Add minimum threshold

* Update LaunchSidechainMasternode.ps1

* Add documentation and refactor transaction manager

* Register message type

* comment ETH Height check

* return loaded eth account

* Refactor transaction originator logic

* Default gas price

* Add status endpoint

* Fix data type

* Pass the calculated fee to the builder (#455)

* Add initial Gnosis Safe interface (#466)

* Add reserve balance logic

* Initial Gnosis Safe interface classes

* Filter (#467)

* Update LaunchSidechainMasternode.ps1

* Update LaunchSidechainMasternode.ps1

* Update ethGasLimit

* Refactor conversion transaction state machine (#473)

* Make existing state numbers the same as previous version

* Fix transfer destination

* Increment agent version

* Fix quorum

* Trap invalid transactionId

* Adjust threshold and agent string

* Fix withdrawalAddresses return type

* Comments, creating logger the new way

* clean usings

* htmldoc for IConversionRequestRepository

* more htmldocs

* Fix tests

* Bump CI

* remove unused InteropRequestRepository

* Update CirrusTest.cs (#491)

* Removed InteropRequestKeyValueStore and InteropRequest bc unused

* fix namespaces

* Move IConversionRequestKeyValueStore and IConversionRequestRepository to the files with implementation

* Move interface and fix namespace

* Post-merge fixes

* [Masternode] Migrate Powershell Script to .NET Console app (#488)

* Add project

* Start and initialize main chain node

* Add main net sync check

* Update NodeController.cs

* Add main and side chain start up checks

* Add collateral wallet checks

* Add wallet fee checks for both chains

* Testing

* Tested up to both nodes syncing and wallet gen

* Add federation key check/generation

* Add monitoring

* Self Review

* Self Review

* Changes based on review

* Fix Tests

* Fixes for PJ (#498)

* [Cirrus] Add ability to have a non-miner join the devmode network (#497)

* Add ability to have a non-miner join devmode network

* Review

* Fix full node event publishing (#499)

* Fix event publishing

* Update FullNode.cs

* Wire in fee distribution

* Fix GetSigOpCount and revert earlier workaround (#493)

* Fix GetSigOpCount and revert earlier workaround

* Add network argument

* Add network argument

* Update FederationWalletManager.cs (#500)

* Bump version

* Fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noescape00 noescape00 noescape00 approved these changes

@zeptin zeptin zeptin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quantumagi @noescape00 @zeptin