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

feat!: provide a compact form of TransactionInput #3460

Merged

Conversation

philipr-za
Copy link
Contributor

Description

This PR updates the TransactionInput struct to support containing the duplicated Output data of the output it is spending OR just the hash of the output. This is done in the original TransactionInput as opposed to creating a new struct to allow for easier reuse of all the code that already supports the TransactionInput.

Methods are provided to:

  • Create a new compact or full TransactionInput
  • Return a compact form of the current TransactionInput
  • Provide the output data that is referenced in the TransactionInput

This PR also updates the RPC definitions and all the conversion methods to support both forms of the TransactionInput

Motivation and Context

The reason for this change is to reduce the amount of duplicate data stored in the Blockchain db and sent across the wire during Block sync. Currently the input is stored with all the duplicated data from the output it is spending along side the spent output in the database which is a duplication of data. Now the TransactionInput is converted into its compact form with just a reference to the spent output before being stored in the database and before being sent across the wire. When the Input is read from the database or received then the associated output data is retrieved from the local datastore.

TODO: apply the compact form of the TransactionInput to transactions submitted to the base node mempool. This will further reduce data sent from wallets to the base nodes.

How Has This Been Tested?

cargo test

@philipr-za philipr-za added the W-consensus_breaking Warn - A change requiring a hard fork to be activated label Oct 15, 2021
@philipr-za philipr-za force-pushed the philip-transaction-input branch from 02f8437 to fb60006 Compare October 18, 2021 06:50
Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

Initial impression is that it looks good, but possibly worth testing on igor or a different network for a bit. Maybe we can set a wire mode?

@philipr-za philipr-za force-pushed the philip-transaction-input branch 3 times, most recently from aa7800c to 28de651 Compare October 25, 2021 09:15
delta1
delta1 previously approved these changes Oct 29, 2021
Copy link
Contributor

@delta1 delta1 left a comment

Choose a reason for hiding this comment

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

Fantastic work 👏

i did some system level testing with a new chain on igor:

  • 2 base nodes
  • 2 wallets
  • Mined with mining node
  • Merge mined with xmrig and proxy
  • Sent and mined transactions between wallets
  • Sent and mined one-sided transactions between wallets

base_layer/core/src/transactions/transaction.rs Outdated Show resolved Hide resolved
This PR updates the TransactionInput struct to support containing the duplicated Output data of the output it is spending OR just the hash of the output. This is done in the original TransactionInput as opposed to creating a new struct to allow for easier reuse of all the code that already supports the TransactionInput.

Methods are provided to:
- Create a new compact or full TransactionInput
- Return a compact form of the current TransactionInput
- Provide the output data that is referenced in the TransactionInput

This PR also updates the RPC definitions and all the conversion methods to support both forms of the TransactionInput

The reason for this change is to reduce the amount of duplicate data stored in the Blockchain db and sent across the wire during Block sync. Currently the input is stored with all the duplicated data from the output it is spending along side the spent output in the database which is a duplication of data. Now the TransactionInput is converted into its compact form with just a reference to the spent output before being stored in the database and before being sent across the wire. When the Input is read from the database or received then the associated output data is retrieved from the local datastore.

TODO: apply the compact form of the TransactionInput to transactions submitted to the base node mempool. This will further reduce data sent from wallets to the base nodes.
@philipr-za philipr-za force-pushed the philip-transaction-input branch from 7b71e2b to 9797116 Compare January 5, 2022 08:45
@stringhandler stringhandler merged commit 834eff9 into tari-project:development Jan 5, 2022
sdbondi added a commit to sdbondi/tari that referenced this pull request Jan 7, 2022
sdbondi added a commit to sdbondi/tari that referenced this pull request Jan 7, 2022
* development:
  chore: remove moving lock.mdb (tari-project#3674)
  chore: merge weatherwax
  feat!: provide a compact form of TransactionInput (tari-project#3460)
  v0.22.1.1
  v0.22.1
  ci: add build step (tari-project#3678)
  fix: edge cases causing bans during header/block sync (tari-project#3661)
  fix: end stale outbound queue immediately on disconnect, retry outbound messages (tari-project#3664)
  feat: add search by commitment to explorer (tari-project#3668)
  feat: tari launchpad (tari-project#3671)
  feat: base_node switching for console_wallet when status is offline (tari-project#3639)
  feat: improve wallet recovery and scanning handling of reorgs (tari-project#3655)
  feat: add GRPC call to search for utxo via commitment hex (tari-project#3666)
  feat: custom_base_node in config (tari-project#3651)
  fix: return correct index for include_pruned_utxos = false (tari-project#3663)
sdbondi added a commit to sdbondi/tari that referenced this pull request Jan 11, 2022
* development: (28 commits)
  feat: covenants implementation (tari-project#3656)
  ci: add tor script to binaries bundle (tari-project#3689)
  chore: remove testnet reset todo in wallet (tari-project#3685)
  feat: dibbler new genesis block with faucet utxos (tari-project#3688)
  ci: fix clippy warning on generated proto module (tari-project#3690)
  test: fix metadata signature cucumber (tari-project#3687)
  refactor!: clean up #testnet reset TODOs (tari-project#3682)
  feat(comms)!: add signature to peer identity to allow third party identity updates (tari-project#3629)
  chore: remove moving lock.mdb (tari-project#3674)
  chore: merge weatherwax
  feat!: provide a compact form of TransactionInput (tari-project#3460)
  fix: allow 0-conf in blockchain db (tari-project#3680)
  v0.22.1.1
  v0.22.1
  ci: add build step (tari-project#3678)
  test: fix cucumber WalletQuery.feature (tari-project#3677)
  test: fix `wait for` step (tari-project#3673)
  fix: edge cases causing bans during header/block sync (tari-project#3661)
  perf(comms)!: optimise connection establishment (tari-project#3658)
  fix: end stale outbound queue immediately on disconnect, retry outbound messages (tari-project#3664)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-consensus_breaking Warn - A change requiring a hard fork to be activated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants