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

Enable payloads for non coinbase transactions #591

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

someone235
Copy link
Contributor

@someone235 someone235 commented Nov 10, 2024

Simpa results

cargo run --release --bin simpa -- --testnet11 -t=5 -n=10000 --long-payload
2024-11-10 17:33:06.044+00:00 [INFO ] Total validation time: 22.023479023s, block processing rate: 454.06 (b/s), transaction processing rate: 2479.35 (t/s)

cargo run --release --bin simpa -- --testnet11 -n=10000
2024-11-10 17:38:16.827+00:00 [INFO ] Total validation time: 14.898869473s, block processing rate: 671.19 (b/s), transaction processing rate: 30463.32 (t/s)

TODO:

  • Make a test for activation logic
  • Make sure txid, txhash and sighash are affected by payload change (maybe we already have tests that verify it, we need to check)
  • Search for TODOs related to payloads in the codebase and apply them if needed
  • Check perf bottlenecks (Maybe the problem is somehow unique to simpa?)
  • Make benchmark for validating a transaction with many inputs and large payloads
  • Launch a network, where txgen creates txns with long payload, and check how the network handles it

@michaelsutton
Copy link
Contributor

Note the implementation of sighash::payload_hash which currently ignores the payload, as well as the TODO within (reused values)

@someone235
Copy link
Contributor Author

Note the implementation of sighash::payload_hash which currently ignores the payload, as well as the TODO within (reused values)

Added to TODOs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants