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

Add tests and helpers related to feature activations #98

Merged
merged 8 commits into from
Sep 10, 2015

Conversation

dexX7
Copy link
Member

@dexX7 dexX7 commented Aug 26, 2015

Add support for RPC "omni_sendactivations" to activate features

The command can be used to activate protocol features, and is going to be added in Omni Core 0.0.10.

Add support for RPC "omni_getactivations"

This command can be used to list pending or completed activations, and is going to be added in Omni Core 0.0.10.

Add specification for the grace period of feature activations

Features are activated at specific block heights, which must be within the range of a grace period to ensure users have enough time to update their clients.

The use of activation commands is restricted to whitelisted senders. To whitelist a source to allow feature activations, the option -omniactivationallowsender="sender" can be used.

The option -omniactivationallowsender=any can be used to remove the restriction.

Add specification for the deactivation of DEx "over-offers"

Once the feature with identifier 5 is active, it is no longer allowed to create offers on the traditional distributed exchange, for an amount that is more than the available balance. During the grace period, the old rules are still in place.

Add specification for the activation of the MetaDEx

Once the feature with identifier 2 is active, the distributed token exchange can be used with non-test tokens. During the grace period, the old rules are still in place.

Add base class for feature activation related tests

The new base class BaseActivationSpec is used to ensure that the client-to-test actually has the required RPCs, or otherwise the tests will be skipped.

Deactivate all feature activation related tests

Feature activation tests currently affect other tests and can only be executed with a prestine state.

This doesn't play well with automated tests, and may not be obvious, when using OmniJ to run tests via an IDE.

The MetaDExActivationSpec only works, if the MetaDEx is not yet activated, which is not the default in Omni Core, even in regtest mode, so it's required to manually un-ignore the specification.

Add workaround to avoid conflict of activations

However, to avoid that the activations affect other tests, a workaround is used to revert consensus affecting changes:

Before the tests, the hash of a "marker block" is stored, and after the tests 50 blocks are mined, and finally the "marker block" is invalidated. This results in a reorganization, and because only the state of the last 50 blocks is persisted, it completely resets the state, including all activations inbetween.

This allows to enable 2/3 activation specs.

The RPC can be used to activate protocol features.
This command can be used to list pending or completed activations.
Features are activated at specific block heights, which must be within
the range of a grace period to ensure users have enough time to update
their clients.

The use of activation commands is restricted to whitelisted senders.

To whitelist a source to allow feature activations, the option
-omniactivationallowsender="sender" can be used.

The feature identifier 3 is currently unused, and a good candidate for
tests.

Note: this test is only successful with a clean state!
Once the feature with identifier 4 is active, it is no longer allowed
to create offers on the traditional distributed exchange, for an amount
that is more than the available balance.

During the grace period, the old rules are still in place.

Note: this test is only successful with a clean state!
Once the feature with identifier 2 is active, the distributed token
exchange can be used with non-test tokens.

During the grace period, the old rules are still in place.

Note: this test is only successful with a clean state, and requires
that the feature is initially disabled!
The new base class "BaseActivationSpec" is used to ensure that the
client-to-test actually has the required RPCs, or otherwise the tests
will be skipped.
Feature activation tests currently affect other tests and can only be
executed with a prestine state.

This doesn't play well with automated tests, and may not be obvious,
when using OmniJ to run tests via an IDE.

The "MetaDExActivationSpec" only works, if the MetaDEx is not yet
activated, which is not the default in Omni Core, even in regtest mode,
so it's required to manually un-ignore the specification.
@dexX7
Copy link
Member Author

dexX7 commented Sep 9, 2015

I updated this PR and removed my old comments, which was not longer applicable.

While the PR still depends on OmniLayer/omnicore#176, I'd consider it as safe to merge, because the tests are skipped, if unsupported.

@msgilligan: feel free to merge, if you consider it as ready at this point. :)

But before doing so, please add omniactivationallowsender=any to the bitcoin.conf, which is used on the CI server for the regtests.

@dexX7 dexX7 changed the title Test feature activation and deactivation of DEx "over-offers" Add tests and helpers related to feature activations Sep 9, 2015
To avoid that the activations affect other tests, a workaround is used
to revert consensus affecting changes:

Before the tests, the hash of a "marker block" is stored, and after the
tests 50 blocks are mined, and finally the "marker block" is
invalidated. This results in a reorganization, and because only the
state of the last 50 blocks is persisted, it completely resets the
state, including all activations inbetween.

This allows to enable 2/3 activation specs.
@zathras-crypto
Copy link

@dexX7 this is excellent - thanks!

msgilligan added a commit that referenced this pull request Sep 10, 2015
Add tests and helpers related to feature activations
@msgilligan msgilligan merged commit 9295779 into OmniLayer:master Sep 10, 2015
@msgilligan
Copy link
Member

@dexX7 I just merged and got the following failure result: https://ci.omni.foundation/job/omni-integ-regtest/160/

Any ideas?

@dexX7
Copy link
Member Author

dexX7 commented Sep 10, 2015

Hm, that's very unexpected. I'm going to test it locally once again soon~.

@msgilligan
Copy link
Member

Let me know. I could turn on @ignore for now, if necessary.

@dexX7
Copy link
Member Author

dexX7 commented Sep 10, 2015

Ahh, it's caused by:

    @Shared Sha256Hash initialBlock // <--- null

    def setupSpec() {
        if (!commandExists('omni_sendactivation')) {
            throw new AssumptionViolatedException('The client has no "omni_sendactivation" command')
        }
        if (!commandExists('omni_getactivations')) { // <--- this throws
            throw new AssumptionViolatedException('The client has no "omni_getactivations" command')
        }

        generateBlock()
        def currentBlockCount = getBlockCount()
        initialBlock = getBlockHash(currentBlockCount) // <--- not reached
    }

    def cleanupSpec() {
        for (int i = 0; i < 50; ++i) {
            generateBlock()
        }
        invalidateBlock(initialBlock) // <--- throws null pointer exception
        clearMemPool()
        generateBlock()
    }

So we'd need a check to ensure initialBlock was actually initialized and isn't null.

How about the following?

diff --git a/omnij-rpc/src/integ/groovy/foundation/omni/test/rpc/activation/BaseActivationSpec.groovy b/omnij-rpc/src/integ/groovy/foundation/omni/test/rpc/activation/BaseActiva
index 5e7a672..9867312 100644
--- a/omnij-rpc/src/integ/groovy/foundation/omni/test/rpc/activation/BaseActivationSpec.groovy
+++ b/omnij-rpc/src/integ/groovy/foundation/omni/test/rpc/activation/BaseActivationSpec.groovy
@@ -50,6 +50,9 @@ abstract class BaseActivationSpec extends BaseRegTestSpec {
     }

     def cleanupSpec() {
+        if (initialBlock == null) {
+            return
+        }
         for (int i = 0; i < 50; ++i) {
             generateBlock()
         }

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.

3 participants