validation: only check input scripts once
Changes PlannedPublic
Actions

Authored by PiRK on Fri, Nov 14, 12:22.

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

Previously, we would check failing input scripts twice when considering
a transaction for the mempool, in order to distinguish policy failures
from consensus failures. This allowed us both to provide a different
error message and to discourage peers for consensus failures. Because we
are no longer discouraging peers for consensus failures during tx relay,
and because checking a script can be expensive, only do this once.

Also renames non-mandatory-script-verify-flag error to
mempool-script-verify-flag-failed.

This is a backport of core#33050
https://github.com/bitcoin/bitcoin/pull/33050/commits/b29ae9efdfeeff774e32ee433ce67d8ed8ecd49f

Depends on D18933

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr33050

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 35067
Build 69594: Build Diff	electrum-functional-tests · electrum-tests · chronik-client-integration-tests · ecash-lib-integration-tests · ecash-agora-integration-tests · lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang · build-chronik-plugins · build-chronik
Build 69593: arc lint + arc unit

Event Timeline

PiRK created this revision.Fri, Nov 14, 12:22

Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Nov 14, 12:22

teamcity edited the summary of this revision. (Show Details)Fri, Nov 14, 12:22

Harbormaster completed remote builds in B35062: Diff 56598.Fri, Nov 14, 12:53

Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Failed tests logs:

====== AgoraPartial enforcedLockTime: AgoraPartial enforcedLockTime AgoraPartial enforcedLockTime ======
AssertionError: expected promise to be rejected with an error including 'Failed getting /broadcast-tx: 400: Br…' but got 'Failed getting /broadcast-tx: 400: Br…'

      + expected - actual

      -Failed getting /broadcast-tx: 400: Broadcast failed: Transaction rejected by mempool: mempool-script-verify-flag-failed (Script failed an OP_EQUALVERIFY operation), input 2 of f9844a277cfe190f3bc9abe30921d6b1e80f6748249e4c3cc342902f0280df23, spending f7a455f0d2cc521ce071223e7f219155f5c4029ef8a11d1c93128ac6a82bff72:1
      +Failed getting /broadcast-tx: 400: Broadcast failed: Transaction rejected by mempool: mandatory-script-verify-flag-failed (Script failed an OP_EQUALVERIFY operation)

Each failure log is accessible here:
AgoraPartial enforcedLockTime: AgoraPartial enforcedLockTime AgoraPartial enforcedLockTime

fix accidental squashing

@bot build-chronik chronik-client-integration-tests

teamcity awarded a token.Fri, Nov 14, 13:19

fix an agora integration test

Harbormaster completed remote builds in B35067: Diff 56604.Fri, Nov 14, 13:56

Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Failed tests logs:

====== AgoraPartial enforcedLockTime: AgoraPartial enforcedLockTime AgoraPartial enforcedLockTime ======
AssertionError: expected promise to be rejected with an error including 'Failed getting /broadcast-tx: 400: Br…' but got 'Failed getting /broadcast-tx: 400: Br…'

      + expected - actual

      -Failed getting /broadcast-tx: 400: Broadcast failed: Transaction rejected by mempool: mempool-script-verify-flag-failed (Script failed an OP_EQUALVERIFY operation), input 2 of f9844a277cfe190f3bc9abe30921d6b1e80f6748249e4c3cc342902f0280df23, spending f7a455f0d2cc521ce071223e7f219155f5c4029ef8a11d1c93128ac6a82bff72:1
      +Failed getting /broadcast-tx: 400: Broadcast failed: Transaction rejected by mempool: mandatory-script-verify-flag-failed (Script failed an OP_EQUALVERIFY operation)

Each failure log is accessible here:
AgoraPartial enforcedLockTime: AgoraPartial enforcedLockTime AgoraPartial enforcedLockTime

Harbormaster completed remote builds in B35068: Diff 56605.Fri, Nov 14, 14:24

rebase

PiRK published this revision for review.Mon, Nov 17, 09:51

Harbormaster completed remote builds in B35084: Diff 56633.Mon, Nov 17, 10:02

@bot ecash-agora-integration-tests chronik-client-integration-tests

teamcity updated a token.Mon, Nov 17, 10:43

The benefit / risk ratio is not clear on that one. The benefits are limited to deduplicating the script checks, and the risks involve making some CPU DoS attacks easier to perform.
This needs more consideration.