Page MenuHomePhabricator

Implement sigops checks in abc-p2p-fullblocktest.py
ClosedPublic

Authored by deadalnix on Jun 12 2017, 22:41.

Details

Summary

This tests the various sigops limit that exist today:

  • 20k per MB rounded up in a block.
  • 20k max per transaction.
Test Plan
../qa/pull-tester/rpc-tests.py abc-p2p-fullblocktest.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
sigopslimit
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 251
Build 251: arc lint + arc unit

Event Timeline

I have tested this and it passed as it stands.
Refer my comments in D182 regarding the block size limit testing.

qa/rpc-tests/abc-p2p-fullblocktest.py
11 ↗(On Diff #413)

It is time to revise the file header to list what is now tested.

250 ↗(On Diff #413)

Just to confirm:

The -1 is due to the coinbase counting for one sigop , bringing the total to MAX_BLOCK_SIGOPS_PER_MB ?

261 ↗(On Diff #413)

Should be "Up to 40k sigops for a block larger than 1MB but <= 2MB"

265 ↗(On Diff #413)

Should be "Not more than 40k sigops for a block > 1MB but <= 2MB"

This revision now requires changes to proceed.Jun 13 2017, 01:07
qa/rpc-tests/abc-p2p-fullblocktest.py
262 ↗(On Diff #413)

We should include a test of another block with the maximum of the acceptable sigops for a 1,000,001 byte block, i.e 2*MAX_BLOCK_SIGOPS_PER_MB - 1

qa/rpc-tests/abc-p2p-fullblocktest.py
250 ↗(On Diff #413)

The custome script is the second output of the coinbase. The first one count as one.

262 ↗(On Diff #413)

Agreed.

deadalnix edited edge metadata.

Add more test cases, make comment more explicits

This revision is now accepted and ready to land.Jun 13 2017, 12:07
This revision was automatically updated to reflect the committed changes.