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: new template with coinbase call #6226

Merged

Conversation

SWvheerden
Copy link
Collaborator

Description

This adds a new grpc call to take a block template and add coinbases to it as desired. The callee is required to ensure that the coinbases amounts provided are correct and per consensus.

Motivation and Context

This is to allow external miners to create a block with 1 sided coinbase utxo's to be added without having to create them as this necessitates the inclusion of all Tari crypto libraries.

How Has This Been Tested?

New unit and cucumber tests

@SWvheerden SWvheerden requested a review from a team as a code owner March 20, 2024 15:38
Copy link

github-actions bot commented Mar 20, 2024

Test Results (CI)

    3 files    120 suites   34m 21s ⏱️
1 270 tests 1 270 ✅ 0 💤 0 ❌
3 802 runs  3 802 ✅ 0 💤 0 ❌

Results for commit d026314.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Mar 20, 2024
Copy link

github-actions bot commented Mar 20, 2024

Test Results (Integration tests)

 2 files  + 2  11 suites  +11   23m 54s ⏱️ + 23m 54s
33 tests +33  31 ✅ +31  0 💤 ±0  2 ❌ +2 
35 runs  +35  33 ✅ +33  0 💤 ±0  2 ❌ +2 

For more details on these failures, see this check.

Results for commit d026314. ± Comparison against base commit e0ad342.

♻️ This comment has been updated with latest results.

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.

I think it is better to just use one call instead of getting the block template, and then adding the coinbases.

One problem is that the fees might be different. In this case I would suggest adding all the fees to the first address supplied.

If we would prefer to do the fees fairly, then we can keep the 2 call approach, but I would suggest setting the timestamp at the same time then.

applications/minotari_app_grpc/proto/base_node.proto Outdated Show resolved Hide resolved
for coinbase in coinbases {
let address = TariAddress::from_hex(&coinbase.address)
.map_err(|e| obscure_error_if_true(report_error_flag, Status::internal(e.to_string())))?;
let range_proof_type = if coinbase.revealed_value_proof {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should only allow revealed values

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we do that, I think we need to make it a consensus rule with this

))
},
};
let gen_hash = handler
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does this get used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In merge mining, its used as the unique identifier for tari

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Mar 27, 2024
@SWvheerden SWvheerden merged commit 148e398 into tari-project:development Mar 27, 2024
14 of 16 checks passed
@SWvheerden SWvheerden deleted the sw_add_coinbase_grpc_call branch April 15, 2024 09:13
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants