Fix vmflags for witness generation 1934. #1961
Conversation
…d enable witness generation in test_blockchain_json test.
| if witness.len() == 0: | ||
| if vmState.stateDB.makeMultiKeys().keys.len() != 0: | ||
| raise newException(ValidationError, "Invalid trie generated from block witness") | ||
| return |
There was a problem hiding this comment.
Returning here because we can't validate an empty witness. The witness should only be empty if the multikeys are empty due to having no transactions in a block.
| @@ -221,6 +227,7 @@ proc importBlock(ctx: var TestCtx, com: CommonRef, | |||
| com, | |||
| tracerInst, | |||
| ) | |||
| ctx.vmState.generateWitness = true # Enable saving witness data | |||
There was a problem hiding this comment.
I'm enabling this for all tests in this file. Will this be a problem because its not covering the default code path where witness generation is disabled? If so I can create a separate test which has the witness generation enabled and leave this one as is.
I guess it depends if there are already other tests covering this.
There was a problem hiding this comment.
I'm assuming this is just an additional code path, and the default is still taken too?
But I don't know this code well at all, @jangko can probably answer better.
There was a problem hiding this comment.
It's ok. Like @kdeme said, the witness generation is is just additional code path. And test_blockchain_json actually don't care about block witness. I piggyback the block witness test in test_blockchain_json because the required code is simply there, instead of crafting another similar test code just to test block witness.
There was a problem hiding this comment.
Ok thanks for the explanation. I'll leave it as is in that case.
Changes included in this PR: