Skip to content

Commit

Permalink
Abort if agent doesn't hold tickets on update keys
Browse files Browse the repository at this point in the history
Addresses a bug identified which allows for a smart contract
to return an update map containing keys that the agent has
not acquired tickets for. This fix ensures that when smart
contract execution completes, all updates will be committed
or none of them will. Previously, committed changes which
the agent held write locks for would be committed, but changes
which never had ticket requests made would silently fail.
Note: if a key has a ticket but is waiting on a lock, the
transaction rolls back and retries. If a ticket has not
been acquired, the transaction should fail as the contract
specifically needs to request locks on important keys.

Signed-off-by: Michael Maurer <[email protected]>
  • Loading branch information
maurermi authored and HalosGhost committed Jan 17, 2024
1 parent 20b1edd commit 3e2a3ef
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
9 changes: 7 additions & 2 deletions src/parsec/agent/impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,13 @@ namespace cbdc::parsec::agent {
if(res.has_value()) {
std::visit(
overloaded{
[&](broker::interface::error_code /* e */) {
m_state = state::commit_failed;
[&](broker::interface::error_code e) {
if(e == broker::interface::error_code::commit_hazard) {
// Do not retry
m_state = state::commit_error;
} else {
m_state = state::commit_failed;
}
m_log->error("Broker error for commit for",
m_ticket_number.value());
m_result = error_code::commit_error;
Expand Down
17 changes: 17 additions & 0 deletions src/parsec/broker/impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,12 +397,29 @@ namespace cbdc::parsec::broker {
return error_code::aborted;
}

auto keys_with_tickets = std::unordered_map<
cbdc::buffer,
bool,
cbdc::hashing::const_sip_hash<cbdc::buffer>>();
for(auto& shard : t_state->m_shard_states) {
for(auto& key : shard.second.m_key_states) {
if(key.second.m_key_state == key_state::locking) {
m_log->error("Cannot commit, still waiting for locks");
return error_code::waiting_for_locks;
}
keys_with_tickets[key.first] = true;
}
}
for(auto& update_key : state_updates) {
if(keys_with_tickets.find(update_key.first)
== keys_with_tickets.end()) {
/// We should fail here, because if this transaction has
/// made it to the commit step without attempting to lock
/// the keys, something signficant has gone wrong, likely
/// associated with the contract itself.
m_log->error("Update map contains keys not associated "
"with tickets. Aborting.");
return error_code::commit_hazard;
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/parsec/broker/interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ namespace cbdc::parsec::broker {
/// Shard error during finish.
finish_error,
/// Shard error during get tickets.
get_tickets_error
get_tickets_error,
/// A commit is attempted without associating update keys with
/// ticket
commit_hazard
};

/// Return type from a begin operation. Either a new ticket number or
Expand Down

0 comments on commit 3e2a3ef

Please sign in to comment.