Skip to content

Commit ca1acd7

Browse files
authored
No fees for rejecting (#4875)
## Motivation We charge fees for all outgoing messages, including bounced ones. That means that a chain with very low balance can become unable to reject tracked messages. The client doesn't expect this, and keeps looping and setting the failing message to `Reject`, even if it already is. ## Proposal Don't charge for bouncing messages. Also, charge only for the length of the operation outcome payload, rather than the length of its serialized size, so that an empty outcome is free, too. Finally, we now break out of the loop in the client if a _rejected_ message fails. ## Test Plan A new test was added that reproduced the issue. ## Release Plan - Nothing to do / These changes follow the usual release cycle. - I will prepare a separate PR for a migration of `testnet_conway` to this new execution semantics. We can decide later whether we want to do that. ## Links - Found in: #4874 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
1 parent 8cf9b80 commit ca1acd7

File tree

4 files changed

+88
-21
lines changed

4 files changed

+88
-21
lines changed

linera-chain/src/block_tracker.rs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ use linera_base::{
1313
};
1414
use linera_execution::{
1515
execution_state_actor::ExecutionStateActor, ExecutionRuntimeContext, ExecutionStateView,
16-
MessageContext, OperationContext, OutgoingMessage, ResourceController, ResourceTracker,
17-
SystemExecutionStateView, TransactionOutcome, TransactionTracker,
16+
MessageContext, MessageKind, OperationContext, OutgoingMessage, ResourceController,
17+
ResourceTracker, SystemExecutionStateView, TransactionOutcome, TransactionTracker,
1818
};
1919
use linera_views::context::Context;
2020
use tracing::instrument;
@@ -287,6 +287,9 @@ impl<'resources, 'blobs> BlockExecutionTracker<'resources, 'blobs> {
287287
let mut resource_controller = self.resource_controller.with_state(view).await?;
288288

289289
for message_out in &txn_outcome.outgoing_messages {
290+
if message_out.kind == MessageKind::Bouncing {
291+
continue; // Bouncing messages are free.
292+
}
290293
resource_controller
291294
.track_message(&message_out.message)
292295
.with_execution_context(context)?;
@@ -357,18 +360,6 @@ impl<'resources, 'blobs> BlockExecutionTracker<'resources, 'blobs> {
357360
.collect()
358361
}
359362

360-
/// Returns the execution context for the current transaction.
361-
pub fn chain_execution_context(&self, transaction: &Transaction) -> ChainExecutionContext {
362-
match transaction {
363-
Transaction::ReceiveMessages(_) => {
364-
ChainExecutionContext::IncomingBundle(self.transaction_index)
365-
}
366-
Transaction::ExecuteOperation(_) => {
367-
ChainExecutionContext::Operation(self.transaction_index)
368-
}
369-
}
370-
}
371-
372363
/// Returns a mutable reference to the resource controller.
373364
pub fn resource_controller_mut(
374365
&mut self,
@@ -399,6 +390,18 @@ impl<'resources, 'blobs> BlockExecutionTracker<'resources, 'blobs> {
399390
self.operation_results,
400391
)
401392
}
393+
394+
/// Returns the execution context for the current transaction.
395+
fn chain_execution_context(&self, transaction: &Transaction) -> ChainExecutionContext {
396+
match transaction {
397+
Transaction::ReceiveMessages(_) => {
398+
ChainExecutionContext::IncomingBundle(self.transaction_index)
399+
}
400+
Transaction::ExecuteOperation(_) => {
401+
ChainExecutionContext::Operation(self.transaction_index)
402+
}
403+
}
404+
}
402405
}
403406

404407
pub(crate) type FinalizeExecutionResult = (

linera-core/src/client/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,26 +1450,29 @@ impl<Env: Environment> Client<Env> {
14501450
.transactions
14511451
.get_mut(*index as usize)
14521452
.expect("Transaction at given index should exist");
1453-
let Transaction::ReceiveMessages(message) = transaction else {
1453+
let Transaction::ReceiveMessages(incoming_bundle) = transaction else {
14541454
panic!(
14551455
"Expected incoming bundle at transaction index {}, found operation",
14561456
index
14571457
);
14581458
};
14591459
ensure!(
1460-
!message.bundle.is_protected(),
1460+
!incoming_bundle.bundle.is_protected(),
14611461
chain_client::Error::BlockProposalError(
14621462
"Protected incoming message failed to execute locally"
14631463
)
14641464
);
1465+
if incoming_bundle.action == MessageAction::Reject {
1466+
return result;
1467+
}
14651468
// Reject the faulty message from the block and continue.
14661469
// TODO(#1420): This is potentially a bit heavy-handed for
14671470
// retryable errors.
14681471
info!(
1469-
%error, origin = ?message.origin,
1470-
"Message failed to execute locally and will be rejected."
1472+
%error, %index, origin = ?incoming_bundle.origin,
1473+
"Message bundle failed to execute locally and will be rejected."
14711474
);
1472-
message.action = MessageAction::Reject;
1475+
incoming_bundle.action = MessageAction::Reject;
14731476
continue;
14741477
}
14751478
}

linera-core/src/unit_tests/client_tests.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2933,3 +2933,48 @@ where
29332933

29342934
Ok(())
29352935
}
2936+
2937+
#[test_case(MemoryStorageBuilder::default(); "memory")]
2938+
#[cfg_attr(feature = "storage-service", test_case(ServiceStorageBuilder::new(); "storage_service"))]
2939+
#[cfg_attr(feature = "rocksdb", test_case(RocksDbStorageBuilder::new().await; "rocks_db"))]
2940+
#[cfg_attr(feature = "dynamodb", test_case(DynamoDbStorageBuilder::default(); "dynamo_db"))]
2941+
#[cfg_attr(feature = "scylladb", test_case(ScyllaDbStorageBuilder::default(); "scylla_db"))]
2942+
#[test_log::test(tokio::test)]
2943+
async fn test_rejected_message_bundles_are_free<B>(storage_builder: B) -> anyhow::Result<()>
2944+
where
2945+
B: StorageBuilder,
2946+
{
2947+
let signer = InMemorySigner::new(None);
2948+
let mut builder = TestBuilder::new(storage_builder, 4, 1, signer)
2949+
.await?
2950+
.with_policy(ResourceControlPolicy::testnet());
2951+
let admin = builder.add_root_chain(1, Amount::from_tokens(2)).await?;
2952+
let user = builder.add_root_chain(1, Amount::ZERO).await?;
2953+
let user_reject = builder
2954+
.make_client_with_options(
2955+
user.chain_id(),
2956+
None,
2957+
BlockHeight::ZERO,
2958+
chain_client::Options {
2959+
message_policy: MessagePolicy::new(BlanketMessagePolicy::Reject, None),
2960+
..chain_client::Options::test_default()
2961+
},
2962+
)
2963+
.await?;
2964+
2965+
let recipient = Account::chain(user.chain_id());
2966+
admin
2967+
.transfer(AccountOwner::CHAIN, Amount::ONE, recipient)
2968+
.await
2969+
.unwrap_ok_committed();
2970+
2971+
user_reject.synchronize_from_validators().await?;
2972+
let (certificates, _) = user_reject.process_inbox().await.unwrap();
2973+
assert_eq!(certificates.len(), 1);
2974+
assert_matches!(
2975+
&certificates[0].block().body.transactions[0],
2976+
Transaction::ReceiveMessages(bundle) if bundle.action == MessageAction::Reject
2977+
);
2978+
2979+
Ok(())
2980+
}

linera-core/src/unit_tests/test_utils.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,11 +1042,12 @@ where
10421042
Ok(self.genesis_storage_builder.build(storage).await)
10431043
}
10441044

1045-
pub async fn make_client(
1045+
pub async fn make_client_with_options(
10461046
&mut self,
10471047
chain_id: ChainId,
10481048
block_hash: Option<CryptoHash>,
10491049
block_height: BlockHeight,
1050+
options: chain_client::Options,
10501051
) -> anyhow::Result<ChainClient<B::Storage>> {
10511052
// Note that new clients are only given the genesis store: they must figure out
10521053
// the rest by asking validators.
@@ -1064,7 +1065,7 @@ where
10641065
format!("Client node for {:.8}", chain_id),
10651066
Duration::from_secs(30),
10661067
Duration::from_secs(1),
1067-
chain_client::Options::test_default(),
1068+
options,
10681069
5_000,
10691070
10_000,
10701071
crate::client::RequestsSchedulerConfig::default(),
@@ -1079,6 +1080,21 @@ where
10791080
))
10801081
}
10811082

1083+
pub async fn make_client(
1084+
&mut self,
1085+
chain_id: ChainId,
1086+
block_hash: Option<CryptoHash>,
1087+
block_height: BlockHeight,
1088+
) -> anyhow::Result<ChainClient<B::Storage>> {
1089+
self.make_client_with_options(
1090+
chain_id,
1091+
block_hash,
1092+
block_height,
1093+
chain_client::Options::test_default(),
1094+
)
1095+
.await
1096+
}
1097+
10821098
/// Tries to find a (confirmation) certificate for the given chain_id and block height.
10831099
pub async fn check_that_validators_have_certificate(
10841100
&self,

0 commit comments

Comments
 (0)