Skip to content

Commit

Permalink
Add calculation of exact encoded transaction as a function in Transac…
Browse files Browse the repository at this point in the history
…tion trait (#2222)

Closes
EspressoSystems/marketplace-builder-core#169
<!-- These comments should help create a useful PR message, please
delete any remaining comments before opening the PR. -->
<!-- If there is no issue number make sure to describe clearly *why*
this PR is necessary. -->
<!-- Mention open questions, remaining TODOs, if any -->

### This PR:
- implement the trait for Transaction defined in sequencer, substitute
all places calculating tx size to the use of this function
- changes are together with
Hotshot changes: EspressoSystems/HotShot#3800
Builder changes:
EspressoSystems/marketplace-builder-core#202
<!-- Describe what this PR adds to this repo and why -->
<!-- E.g. -->
<!-- * Implements feature 1 -->
<!-- * Fixes bug 3 -->

### This PR does not:
<!-- Describe what is out of scope for this PR, if applicable. Leave
this section blank if it's not applicable -->
<!-- This section helps avoid the reviewer having to needlessly point
out missing parts -->
<!-- * Implement feature 3 because that feature is blocked by Issue 4
-->
<!-- * Implement xyz because that is tracked in issue #123. -->
<!-- * Address xzy for which I opened issue #456 -->

### Key places to review:
<!-- Describe key places for reviewers to pay close attention to -->
<!-- * file.rs, `add_integers` function -->
<!-- Or directly comment on those files/lines to make it easier for the
reviewers -->

<!-- ### How to test this PR:  -->
<!-- Optional, uncomment the above line if this is relevant to your PR
-->
<!-- If your PR is fully tested through CI there is no need to add this
section -->
<!-- * E.g. `just test` -->

<!-- ### Things tested -->
<!-- Anything that was manually tested (that is not tested in CI). -->
<!-- E.g. building/running of docker containers. Changes to docker demo,
... -->
<!-- Especially mention anything untested, with reasoning and link an
issue to resolve this. -->

<!-- Complete the following items before creating this PR -->
<!-- [ ] Issue linked or PR description mentions why this change is
necessary. -->
<!-- [ ] PR description is clear enough for reviewers. -->
<!-- [ ] Documentation for changes (additions) has been updated (added).
-->
<!-- [ ] If this is a draft it is marked as "draft".  -->

<!-- To make changes to this template edit
https://github.com/EspressoSystems/.github/blob/main/PULL_REQUEST_TEMPLATE.md
-->
  • Loading branch information
dailinsubjam authored Oct 29, 2024
2 parents efcc5c1 + 9bf604d commit 0801a2d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
9 changes: 2 additions & 7 deletions sequencer/src/block/full_payload/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,8 @@ impl Payload {
// add each tx to its namespace
let mut ns_builders = BTreeMap::<NamespaceId, NsPayloadBuilder>::new();
for tx in transactions.into_iter() {
// accounting for block byte length limit
block_byte_len += tx.payload().len() + NsPayloadBuilder::tx_table_entry_byte_len();
if !ns_builders.contains_key(&tx.namespace()) {
// each new namespace adds overhead
block_byte_len +=
NsTableBuilder::entry_byte_len() + NsPayloadBuilder::tx_table_header_byte_len();
}
// accounting for block byte length limit
block_byte_len += tx.size_in_block(!ns_builders.contains_key(&tx.namespace()));
if block_byte_len > max_block_byte_len {
tracing::warn!("transactions truncated to fit in maximum block byte length {max_block_byte_len}");
break;
Expand Down
18 changes: 6 additions & 12 deletions types/src/v0/impls/block/full_payload/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ use jf_vid::VidScheme;
use sha2::Digest;
use thiserror::Error;

use crate::Transaction;
use crate::{
v0::impls::{NodeState, ValidatedState},
v0_1::ChainConfig,
Index, Iter, NamespaceId, NsIndex, NsPayload, NsPayloadBuilder, NsPayloadRange, NsTable,
NsTableBuilder, Payload, PayloadByteLen, SeqTypes, Transaction, TxProof,
NsTableBuilder, Payload, PayloadByteLen, SeqTypes, TxProof,
};

#[derive(serde::Deserialize, serde::Serialize, Error, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -79,23 +80,16 @@ impl Payload {
<Self as BlockPayload<SeqTypes>>::Error,
> {
// accounting for block byte length limit
let max_block_byte_len: usize = u64::from(chain_config.max_block_size)
.try_into()
.expect("too large max block size for architecture");
let mut block_byte_len = NsTableBuilder::header_byte_len();
let max_block_byte_len = u64::from(chain_config.max_block_size);
let mut block_byte_len = NsTableBuilder::header_byte_len() as u64;

// add each tx to its namespace
let mut ns_builders = BTreeMap::<NamespaceId, NsPayloadBuilder>::new();
for tx in transactions.into_iter() {
let mut tx_size = tx.payload().len() + NsPayloadBuilder::tx_table_entry_byte_len();
if !ns_builders.contains_key(&tx.namespace()) {
// each new namespace adds overhead
tx_size +=
NsTableBuilder::entry_byte_len() + NsPayloadBuilder::tx_table_header_byte_len();
}
let tx_size = tx.size_in_block(!ns_builders.contains_key(&tx.namespace()));

if tx_size > max_block_byte_len {
// skip this transaction since it excceds the block size limit
// skip this transaction since it exceeds the block size limit
tracing::warn!(
"skip the transaction to fit in maximum block byte length {max_block_byte_len}, transaction size {tx_size}"
);
Expand Down
22 changes: 21 additions & 1 deletion types/src/v0/impls/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use serde::{de::Error, Deserialize, Deserializer};

use crate::{NamespaceId, Transaction};

use super::{NsPayloadBuilder, NsTableBuilder};

impl From<u32> for NamespaceId {
fn from(value: u32) -> Self {
Self(value as u64)
Expand Down Expand Up @@ -60,6 +62,16 @@ impl Transaction {
self.payload
}

pub fn size_in_block(&self, new_ns: bool) -> u64 {
if new_ns {
// each new namespace adds overhead
// here self.minimum_block_size() = `self.payload().len() + NsPayloadBuilder::tx_table_entry_byte_len() + NsTableBuilder::entry_byte_len() + NsPayloadBuilder::tx_table_header_byte_len()`
self.minimum_block_size()
} else {
(self.payload().len() + NsPayloadBuilder::tx_table_entry_byte_len()) as u64
}
}

#[cfg(any(test, feature = "testing"))]
pub fn random(rng: &mut dyn rand::RngCore) -> Self {
use rand::Rng;
Expand All @@ -79,7 +91,15 @@ impl Transaction {
}
}

impl HotShotTransaction for Transaction {}
impl HotShotTransaction for Transaction {
fn minimum_block_size(&self) -> u64 {
let len = self.payload().len()
+ NsPayloadBuilder::tx_table_entry_byte_len()
+ NsTableBuilder::entry_byte_len()
+ NsPayloadBuilder::tx_table_header_byte_len();
len as u64
}
}

impl Committable for Transaction {
fn commit(&self) -> Commitment<Self> {
Expand Down

0 comments on commit 0801a2d

Please sign in to comment.