remove assumption that coinbase scriptSig prefix contains only BIP34 #1461
Conversation
this is a more generic name, as this extra data could be used for other things and not only pool signature note: this renaming only happens on `roles_logic_sv2`, but not on SRI application layer (`roles`), where the field remains called `pool_signature`
plebhash
force-pushed
the
dont-assume-script-sig-bip34
branch
3 times, most recently
from
7e085b6 to
b0f0257
Compare
plebhash
force-pushed
the
dont-assume-script-sig-bip34
branch
from
b0f0257 to
e59e1e7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1461 +/- ##
==========================================
+ Coverage 16.71% 18.84% +2.13%
==========================================
Files 155 157 +2
Lines 11133 11217 +84
==========================================
+ Hits 1861 2114 +253
+ Misses 9272 9103 -169
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| // witness | ||
| let witness = match bip34_bytes.len() { | ||
| ) -> Result<Transaction, Error> { | ||
| // outside of test environments, we need to make sure the BIP34 data is sane |
There was a problem hiding this comment.
I would remove this test, I don't see why we need to check this here.
There was a problem hiding this comment.
this is to make sure TP sent the correct BIP34 bytes
otherwise, what's the purpose of the Error::InvalidBip34Bytes variant?
and most importantly: what happens if TP sends a scriptSig with bad BIP34 bytes?
There was a problem hiding this comment.
mmm not sure that we should verify if a coin-base is valid or not here. The TP must send valid data otherwise nothing work, so we should assume that is valid IMO. Still IMO is the TP task make sure that templates are good not downstream task.
close #1460
builds on top of #1442