Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCH Nov2018 HF: min tx size is 100 bytes #526

Merged
merged 4 commits into from
Nov 12, 2018
Merged

Conversation

svyatonik
Copy link
Contributor

@svyatonik svyatonik commented Oct 16, 2018

on top of #525
these commits only: ad4f51a + 18a2766
part of #519

This PR introduces new consensus rule - all post-HF transactions must be > than 100 bytes.

P.S. we should have some kind of trigger which is fired when HF block appears + clears the memory pool. Otherwise memory pool could contain invalid transactions. (submitted an issue #527)

@svyatonik svyatonik added A0-pleasereview Pull request needs code review. M4-core Core client code / Rust. labels Oct 16, 2018
Copy link

@dagurval dagurval left a comment

Choose a reason for hiding this comment

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

I'm not familiar enough with the code base to verify this, so I'll ask

  • Do you also verify that the coinbase is >= 100 bytes?
  • Do you at some point create a coinbase transaction in the codebase? In that case, you'll need to make sure it's the right size

}
}

fn check(&self) -> Result<(), TransactionError> {
Copy link

Choose a reason for hiding this comment

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

Nit: Can you void a call to serialized_size by checking if self.min_transaction_size == 0 first?

@svyatonik
Copy link
Contributor Author

@dagurval Appreciate your review! Fixed a nit. Regarding coinbase: (1) this check affects coinbase (2) we only support getblocktemplate RPC which returns coinbasevalue, but not the coinbasetxn. So coinbase is composed externally.

@svyatonik svyatonik merged commit 0d8edaa into master Nov 12, 2018
@svyatonik svyatonik deleted the bch_nov2018_min_tx_size branch November 12, 2018 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. M4-core Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants