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

Optional transaction payload for the /v2/fees/transaction RPC endpoint #5839

Open
zone117x opened this issue Feb 14, 2025 · 0 comments
Open
Assignees

Comments

@zone117x
Copy link
Member

zone117x commented Feb 14, 2025

The current POST /v2/fees/transaction endpoint takes the following parameters:

  • transaction_payload is a hex-encoded serialization of the TransactionPayload for the transaction (note: not the fully serialized tx)
  • estimated_len is an optional argument that provides the endpoint

When the estimated_len value is omitted, a simple heuristic is used to guess the full tx length (it just assumes the most simple tx type, e.g. single-sig auth).

We're seeing clients tend to omit the estimated_len value (path of least resistance), and the heuristic sometimes failing. Ideally clients should always provide the estimated_len and this wouldn't be an issue. However, we could make a backward-compatible change to this endpoint to make the path of least resistance "just work", by allowing the clients to submit the fully serialized tx rather than just the payload.

A simple approach is to modify this same POST endpoint in backward-compatible way where a transaction property can be specified instead of only a transaction_payload, i.e.

Before:

#[derive(Serialize, Deserialize)]
pub struct FeeRateEstimateRequestBody {
    #[serde(default)]
    pub estimated_len: Option<u64>,
    pub transaction_payload: String,
}

After:

#[derive(Serialize, Deserialize, Debug)]
pub struct FeeRateEstimateRequestBody {
    #[serde(default)]
    pub estimated_len: Option<u64>,
    
    #[serde(alias = "transaction")]
    pub transaction_payload: String,
}

... and then modify these lines to try to deserialize a full tx if the tx payload deser fails, and get the actual full tx length:

let tx = TransactionPayload::consensus_deserialize(&mut payload_data.as_slice())?;
let estimated_len =
std::cmp::max(body.estimated_len.unwrap_or(0), payload_data.len() as u64);

Some clients/partners already have a workflow that looks something like:

  1. Generate and serialize a tx ready to submit
  2. Extract the payload bytes
  3. Request fee estimate
  4. Re-generate tx with new fee and re-sign
  5. Submit the tx

With this RPC change, they can continue to use that workflow but simplify it with the removal of step 2.

Security concerns

We don't want to encourage clients to submit fully signed txs to public/untrusted nodes for the purpose of fee estimation. So we should enforce that the serialized tx is unsigned (null bytes for the sig, or just an invalid sig). Many partners/exchanges run their own local trusted node and don't care about this issue, so bonus of the change included a config option that could disable this check. Then they wouldn't have to change their existing workflows if they are already only serializing fully-signed txs.

@zone117x zone117x self-assigned this Feb 14, 2025
@github-project-automation github-project-automation bot moved this to Status: 🆕 New in Stacks Core Eng Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: 🆕 New
Development

No branches or pull requests

1 participant