Skip to content

Conversation

naiza2000
Copy link
Contributor

Follow-up to #22253 adding changes suggested in #22253 (review)

@DrahtBot DrahtBot added the Tests label Jul 22, 2021
@glozow
Copy link
Member

glozow commented Jul 23, 2021

Concept ACK, thanks for picking this up! Needs a rebase and squash.

@brunoerg
Copy link
Contributor

@michaelfolkson gave me a tip in my first PR that might be helpful for you:

You can do a reset on your branch to the current top commit on master, then re-adding your commit and force pushing.

git reset --hard insert_current_top_commit_hash_on_master

Then add a single commit on top and force pushing

@jnewbery
Copy link
Contributor

Concept ACK

Did you consider also doing:

I think the simplest parent witness script to create same-nonwitness-data txs would be OP_DROP OP_TRUE, with a different arbitrary witness stack element in each child transaction. OTOH multiple script branches are far more realistic, I guess that's the primary scenario :)

(#22253 (review))

@theStack
Copy link
Contributor

Concept ACK

Thanks for picking this up and welcome as a new contributor! 🎉
Happy to review/test as soon as the CI is green :)

@glozow
Copy link
Member

glozow commented Aug 2, 2021

(Also needs a squash)

Improve mempool_accept_wtxid.py

Improve mempool_accept_wtxid.py

Improve mempool_accept_wtxid.py

Improve mempool_accept_wtxid.py
Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 91b0597

@maflcko maflcko merged commit 87257d8 into bitcoin:master Aug 3, 2021
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post-merge ACK 91b0597 🍺

nit: not a big issue, but next time, please take care of the commit message body, the 4x"Improve mempool_accept_wtxid.py" doesn't contain any information :)

@naiza2000
Copy link
Contributor Author

naiza2000 commented Aug 3, 2021

post-merge ACK 91b0597 🍺

nit: not a big issue, but next time, please take care of the commit message body, the 4x"Improve mempool_accept_wtxid.py" doesn't contain any information :)

Oh yes, thank you for the guidance. Will certainly take care of that in the future.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 4, 2021
91b0597 Improve mempool_accept_wtxid.py (naiza)

Pull request description:

  Follow-up to bitcoin#22253 adding changes suggested in [bitcoin#22253 (review)](bitcoin#22253 (comment))

ACKs for top commit:
  glozow:
    utACK 91b0597

Tree-SHA512: 383064138a5b2160d769c9df370470fd585c91682083013a6fa15e14448a4b481bc09b3a0ed6e75554db2c378df6b2263c65f209f973c9e9d577e15814a4be1d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants