Please use self.tip = int(self.nodes[0].getbestblockhash(), 16) instead. I know there's some old code that still uses this...

This refactor can be applied as a separate diff to help speed up review of this diff.

Update codename to great wall

Prefer using int(self.hash, 16) instead

coinbaes => coinbase

GREAT_WALL_ACTIVATION_TIME * 2 instead, then you can remove IN_THE_FUTURE, which is unnecessary to have a separate definition.

The tests shouldn't care about the actual activation time. It's an unnecessary dependency. Look at other tests for *_ACTIVATION_TIME and you'll see them set to arbitrarily large values like 9999999. This should be sufficient to set a value like this in all tests where it's needed.

Please use the format method (e.g. '-greatwallactivationtime={}'.format(GREAT_WALL_ACTIVATION_TIME)) instead of % for string formatting

Please chose a fixed value so the test is repeatable.

Having a linter yelling at people for this would be beneficial. % syntax is so much nicer, idk why pything is deprecating it :(

@dagurval is correct. However, it indeed needs to be further in the future than GREAT_WALL_ACTIVATION_TIME is.

The structure of this test is very confusing.

Here is a good test plan:

1. Check that the tx is accepted, both when sent to the mempool and in blocks (you may want to disable the standardness check).
2. Move the MTP to GREAT_WALL_ACTIVATION_TIME - 1
3. Check that you get the same as 1.
4. Move the MTP to GREAT_WALL_ACTIVATION_TIME
5. Check that the tx is rejected from the mempool.
6. Check that the tx cannot be mined and it is accepted in a block.
7. invalid another block.
8. tx can be sent back in the mempool.

You can use tests for the activation of the last fork as examples. Very tight testing is required to detect problem such as off by one errors. You probably want to have two nodes, one set to accept non standard txns, and one set to reject. You also want to check that node do nto ban each others when one activated teh fork and one did not.

Just launch the node with GREAT_WALL_ACTIVATION_TIME set absurdly far away in the future.

No.

While I have nothing aainst these per se, mixing refactoring with logic change makes the hole changeset very hard to work with.

Please split refactoring out of this diff so we can see clearly what logic changed.

Isn't this completely redundant ?

Choosing a value that has nothing to do with anything in the cpp codebase ensure that this doesn't just happen to be working for reasons others than what we expect. For instance, using this, if the command line parameter do not work, we wouldn't know.

activation_tests.cpp actually checks that the trigger is set at the right time.