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

Add secure minting #110

Draft
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 2pc-compose.cfg
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
2pc=1
minter_count=1
minter0="1f05f6173c4f7bef58f7e912c4cb1389097a38f1a9e24c3674d67a0f142af244"
sentinel_count=1
sentinel0_endpoint="sentinel0:5555"
sentinel0_loglevel="WARN"
Expand Down
2 changes: 2 additions & 0 deletions 2pc.cfg.sample
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
2pc=1
minter_count=1
minter0="1f05f6173c4f7bef58f7e912c4cb1389097a38f1a9e24c3674d67a0f142af244"
sentinel_count=1
sentinel0_endpoint="127.0.0.1:5557"
sentinel0_loglevel="WARN"
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ Additionally, you can start the atomizer architecture by passing `--file docker-
## Setup test wallets and test them

The following commands are all performed from within the second container we started in the previous step.

* First, make a demo wallet for minting coins
```terminal
# ./build/src/uhs/client/client-cli wallet0.dat demowallet
```

In each of the below commands, you should pass `atomizer-compose.cfg` instead of `2pc-compose.cfg` if you started the atomizer architecture.

* Mint new coins (e.g., 10 new UTXOs each with a value of 5 atomic units of currency)
Expand Down
2 changes: 2 additions & 0 deletions atomizer-compose.cfg
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
minter_count=1
minter0="1f05f6173c4f7bef58f7e912c4cb1389097a38f1a9e24c3674d67a0f142af244"
atomizer_count=1
atomizer0_endpoint="atomizer0:5555"
atomizer0_raft_endpoint="atomizer0:6666"
Expand Down
2 changes: 2 additions & 0 deletions multimachine.cfg.sample
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
minter_count=1
minter0="1f05f6173c4f7bef58f7e912c4cb1389097a38f1a9e24c3674d67a0f142af244"
atomizer_count=3
atomizer0_endpoint="192.168.1.2:5555"
atomizer0_raft_endpoint="192.168.1.2:6666"
Expand Down
1 change: 1 addition & 0 deletions scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ run_test_suite () {
}

echo "Running unit tests..."
cp tests/unit/*.cfg $BUILD_DIR
run_test_suite "tests/unit/run_unit_tests" "unit_tests_coverage"

echo "Running integration tests..."
Expand Down
16 changes: 13 additions & 3 deletions src/uhs/atomizer/sentinel/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace cbdc::sentinel {

auto controller::execute_transaction(transaction::full_tx tx)
-> std::optional<cbdc::sentinel::execute_response> {
const auto res = transaction::validation::check_tx(tx);
const auto res = transaction::validation::check_tx(tx, m_opts);
HalosGhost marked this conversation as resolved.
Show resolved Hide resolved
tx_status status{tx_status::pending};
if(res.has_value()) {
status = tx_status::static_invalid;
Expand Down Expand Up @@ -118,7 +118,7 @@ namespace cbdc::sentinel {

auto controller::validate_transaction(transaction::full_tx tx)
-> std::optional<validate_response> {
const auto res = transaction::validation::check_tx(tx);
const auto res = transaction::validation::check_tx(tx, m_opts);
if(res.has_value()) {
return std::nullopt;
}
Expand Down Expand Up @@ -167,7 +167,17 @@ namespace cbdc::sentinel {
return;
}

send_compact_tx(ctx);
// If the tx has no inputs, it's a mint. Send it directly to one of the
HalosGhost marked this conversation as resolved.
Show resolved Hide resolved
// shards
if(ctx.m_inputs.empty()) {
auto ctx_pkt
= std::make_shared<cbdc::buffer>(cbdc::make_buffer(ctx));
if(!m_shard_network.send_to_one(ctx_pkt)) {
m_logger->error("Failed to send mint tx to shard");
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Style point: prefer return; over else here to avoid the unnecessary condition nesting (which causes a cognitive complexity penalty).

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f56-avoid-unnecessary-condition-nesting

send_compact_tx(ctx);
}
}

void controller::send_compact_tx(const transaction::compact_tx& ctx) {
Expand Down
10 changes: 6 additions & 4 deletions src/uhs/atomizer/shard/shard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,13 @@ namespace cbdc::shard {
cbdc::watchtower::tx_error_sync{}};
}

atomizer::tx_notify_request msg;

// If the tx has no inputs, it's a mint.
if(tx.m_inputs.empty()) {
return cbdc::watchtower::tx_error{
tx.m_id,
cbdc::watchtower::tx_error_inputs_dne{{}}};
msg.m_tx = std::move(tx);
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need the sentinel attestations here?

m_attestations is not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@metalicjames This is related to an issue that I asked about early in the process of working on this. The Atomizer, unlike 2PC, actually requires inputs in the compact transaction. A mint transaction doesn't have any inputs. So making mint transactions work with the Atomized feels like a hack and deviates from the normal transaction flow. Any thoughts or insights you have on this would be appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I think the atomizer should be able to understand mint transactions. Otherwise, how would the mint transaction be included in a block and applied by the shards? If the transaction is included in a block, the atomizer should check the sentinel attestations. A valid mint transaction should still be validated by multiple sentinels and attested to, the same way as any other transaction. Otherwise, how can we be confident the mint transaction wasn't validated by a malicious sentinel, ignoring the authorized minter keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@metalicjames A mint transaction flows through the sentinel and accumulates attestations the same as any other transaction. The controller checks the attestations before it forwards it to the shard: https://github.com/mit-dci/opencbdc-tx/blob/trunk/src/uhs/atomizer/shard/controller.cpp#L152. All the shard appears to do is collect the input index into the msg attestations set https://github.com/mit-dci/opencbdc-tx/blob/trunk/src/uhs/atomizer/shard/shard.cpp#L151. What concerns me more is the fact that atomizer allow this (at least passes all tests) even though the msg.m_attestations is empty for a mint transaction.

msg.m_block_height = best_block_height();
HalosGhost marked this conversation as resolved.
Show resolved Hide resolved
return msg;
}

auto read_options = m_read_options;
Expand Down Expand Up @@ -158,7 +161,6 @@ namespace cbdc::shard {
cbdc::watchtower::tx_error_inputs_dne{dne_inputs}};
}

atomizer::tx_notify_request msg;
msg.m_attestations = std::move(attestations);
msg.m_tx = std::move(tx);
msg.m_block_height = snp_height;
Expand Down
129 changes: 89 additions & 40 deletions src/uhs/client/client-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
#include "crypto/sha256.h"
#include "twophase_client.hpp"
#include "uhs/transaction/messages.hpp"
#include "uhs/transaction/wallet.hpp"
#include "util/common/config.hpp"
#include "util/serialization/util.hpp"

#include <filesystem>
#include <future>
#include <iostream>

Expand All @@ -31,10 +33,50 @@ auto mint_command(cbdc::client& client, const std::vector<std::string>& args)
const auto n_outputs = std::stoull(args[5]);
const auto output_val = std::stoul(args[6]);

const auto mint_tx
const auto [tx, resp]
= client.mint(n_outputs, static_cast<uint32_t>(output_val));
std::cout << cbdc::to_string(cbdc::transaction::tx_id(mint_tx))
if(!tx.has_value()) {
std::cout << "Could not generate valid mint tx." << std::endl;
return false;
}

std::cout << "tx_id:" << std::endl
<< cbdc::to_string(cbdc::transaction::tx_id(tx.value()))
<< std::endl;

if(resp.has_value()) {
Copy link
Member

Choose a reason for hiding this comment

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

This code seems duplicated from print_tx_result, can we refactor it?

std::cout << "Sentinel responded: "
<< cbdc::sentinel::to_string(resp.value().m_tx_status)
<< std::endl;
if(resp.value().m_tx_error.has_value()) {
std::cout << "Validation error: "
<< cbdc::transaction::validation::to_string(
resp.value().m_tx_error.value())
<< std::endl;
}
}

return true;
}

/// Generate a demo wallet for use with demo. It creates a minter public key
/// to match the value pre-configued in the provided 'cfg' files.
///
/// Example use: client-cli 'wallet file name' demowallet
auto generate_demo_minter_wallet(const std::vector<std::string>& args)
-> bool {
const auto wallet_file = args[1];
if(std::filesystem::exists(wallet_file)) {
std::cout << " " << wallet_file << " already exists" << std::endl;
return false;
}
cbdc::transaction::wallet wallet{};
const auto pk = wallet.generate_test_minter_key();
const auto hexed = cbdc::to_string(pk);
wallet.save(wallet_file);

std::cout << " Created demo wallet. Saved to: " << wallet_file << "\n"
<< " Minter public key is: " << hexed << std::endl;
return true;
}

Expand Down Expand Up @@ -204,9 +246,46 @@ auto confirmtx_command(cbdc::client& client,
return true;
}

auto dispatch_command(const std::string& command,
cbdc::client& client,
const std::vector<std::string>& args) -> bool {
auto result = true;
if(command == "mint") {
result = mint_command(client, args);
} else if(command == "send") {
result = send_command(client, args);
} else if(command == "fan") {
result = fan_command(client, args);
} else if(command == "sync") {
client.sync();
} else if(command == "newaddress") {
newaddress_command(client);
} else if(command == "info") {
const auto balance = client.balance();
const auto n_txos = client.utxo_count();
std::cout << "Balance: " << cbdc::client::print_amount(balance)
<< ", UTXOs: " << n_txos
<< ", pending TXs: " << client.pending_tx_count()
<< std::endl;
} else if(command == "importinput") {
result = importinput_command(client, args);
} else if(command == "confirmtx") {
result = confirmtx_command(client, args);
} else {
std::cerr << "Unknown command" << std::endl;
}
return result;
}

// LCOV_EXCL_START
auto main(int argc, char** argv) -> int {
auto args = cbdc::config::get_args(argc, argv);

// Create a demo wallet
if(args.size() == 3 && args[2] == "demowallet") {
HalosGhost marked this conversation as resolved.
Show resolved Hide resolved
return generate_demo_minter_wallet(args) ? 0 : -1;
}

static constexpr auto min_arg_count = 5;
if(args.size() < min_arg_count) {
std::cerr << "Usage: " << args[0]
Expand All @@ -215,7 +294,12 @@ auto main(int argc, char** argv) -> int {
return 0;
}

auto cfg_or_err = cbdc::config::load_options(args[1]);
const auto config_file = args[1];
const auto client_file = args[2];
const auto wallet_file = args[3];
const auto command = args[4];

auto cfg_or_err = cbdc::config::load_options(config_file);
if(std::holds_alternative<std::string>(cfg_or_err)) {
std::cerr << "Error loading config file: "
<< std::get<std::string>(cfg_or_err) << std::endl;
Expand All @@ -226,9 +310,6 @@ auto main(int argc, char** argv) -> int {

SHA256AutoDetect();

const auto wallet_file = args[3];
const auto client_file = args[2];

auto logger = std::make_shared<cbdc::logging::log>(
cbdc::config::defaults::log_level);

Expand All @@ -249,40 +330,8 @@ auto main(int argc, char** argv) -> int {
return -1;
}

const auto command = std::string(args[4]);
if(command == "mint") {
if(!mint_command(*client, args)) {
return -1;
}
} else if(command == "send") {
if(!send_command(*client, args)) {
return -1;
}
} else if(command == "fan") {
if(!fan_command(*client, args)) {
return -1;
}
} else if(command == "sync") {
client->sync();
} else if(command == "newaddress") {
newaddress_command(*client);
} else if(command == "info") {
const auto balance = client->balance();
const auto n_txos = client->utxo_count();
std::cout << "Balance: " << cbdc::client::print_amount(balance)
<< ", UTXOs: " << n_txos
<< ", pending TXs: " << client->pending_tx_count()
<< std::endl;
} else if(command == "importinput") {
if(!importinput_command(*client, args)) {
return -1;
}
} else if(command == "confirmtx") {
if(!confirmtx_command(*client, args)) {
return -1;
}
} else {
std::cerr << "Unknown command" << std::endl;
if(!dispatch_command(command, *client, args)) {
return -1;
}

// TODO: check that the send queue has drained before closing
Expand Down
22 changes: 16 additions & 6 deletions src/uhs/client/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,29 @@ namespace cbdc {
return ss.str();
}

auto client::mint_for_testing(size_t n_outputs, uint32_t output_val)
-> std::pair<std::optional<transaction::full_tx>,
std::optional<cbdc::sentinel::execute_response>> {
m_wallet.generate_test_minter_key();
return mint(n_outputs, output_val);
}

auto client::mint(size_t n_outputs, uint32_t output_val)
-> transaction::full_tx {
-> std::pair<std::optional<transaction::full_tx>,
std::optional<cbdc::sentinel::execute_response>> {
static constexpr auto null_return
= std::make_pair(std::nullopt, std::nullopt);

auto mint_tx = m_wallet.mint_new_coins(n_outputs, output_val);
import_transaction(mint_tx);

// TODO: make a formal way of minting. For now bypass the sentinels.
if(!send_mint_tx(mint_tx)) {
m_logger->error("Failed to send mint tx");
auto res = send_transaction(mint_tx);
if(!res.has_value()) {
return null_return;
}

save();

return mint_tx;
return std::make_pair(mint_tx, res.value());
}

void client::sign_transaction(transaction::full_tx& tx) {
Expand Down
20 changes: 18 additions & 2 deletions src/uhs/client/client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ namespace cbdc {
/// \return USD formatted value.
static auto print_amount(uint64_t val) -> std::string;

/// \brief Mint coins for testing.
///
/// Provides a pre-calculated keypair to be used for configuration
/// files for testing and demo environments. Use the hexed public key:
/// 1f05f6173c4f7bef58f7e912c4cb1389097a38f1a9e24c3674d67a0f142af244 as
/// the value for minter0 in a configuration file.
/// \param n_outputs number of new spendable outputs to create.
/// \param output_val value of the amount to associate with each output in the base unit of the currency.
/// \return the transaction and sentinel response.
auto mint_for_testing(size_t n_outputs, uint32_t output_val)
-> std::pair<std::optional<transaction::full_tx>,
std::optional<cbdc::sentinel::execute_response>>;

/// \brief Creates the specified number spendable outputs each with the
/// specified value.
///
Expand All @@ -57,9 +70,10 @@ namespace cbdc {
/// transaction to the system via \ref send_mint_tx.
/// \param n_outputs number of new spendable outputs to create.
/// \param output_val value of the amount to associate with each output in the base unit of the currency.
/// \return the completed transaction.
/// \return the transaction and sentinel response.
auto mint(size_t n_outputs, uint32_t output_val)
-> transaction::full_tx;
-> std::pair<std::optional<transaction::full_tx>,
std::optional<cbdc::sentinel::execute_response>>;

/// \brief Send a specified amount from this client's wallet to a
/// target address.
Expand Down Expand Up @@ -237,6 +251,8 @@ namespace cbdc {
/// \brief Sends the given minting transaction to a service that will
/// accept and process it.
///
/// TODO: Remove. No longer needed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the point of leaving this just to minimize the change in this PR? (it's otherwise leaving specifically-dead code in the repo.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HalosGhost I primarily left it in the event someone objected to the changes and/or wanted the functionality for other reasons (testing?).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing occurs to me on why it would still be needed with the adopting of this (pretty simple even) minting mechanism, but perhaps @metalicjames will think otherwise (I'm open to hearing alternative opinions). :)

Copy link
Member

Choose a reason for hiding this comment

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

If we're not using it, let's remove it.

///
/// Called by \ref mint to send the resulting transaction. Subclasses
/// should define custom transmission logic here.
/// \param mint_tx invalid transaction that mints new coins.
Expand Down
Loading