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 testmempoolaccept endpoint #81

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Conversation

mononaut
Copy link
Contributor

This PR adds a new REST API endpoint POST /txs/test, which accepts a comma-separated list of transactions as raw hex strings, passes them to testmempoolaccept, and returns the results as JSON.

src/daemon.rs Show resolved Hide resolved
@junderw
Copy link
Member

junderw commented Mar 23, 2024

Q: Where do you plan to use this endpoint and how?

@mononaut
Copy link
Contributor Author

mononaut commented Mar 23, 2024

Access to testmempoolaccept is a frequent feature request from some of our API users, but I'll probably also add an interface to the mempool frontend similar to the /tx/push page.

@junderw
Copy link
Member

junderw commented Mar 23, 2024

Yeah, if we're going to offer this publicly, we should definitely offer an optional maxfeerate parameter.

Might be worth it to check the request body to make sure it's only hex and commas... and maybe we should limit it somehow. (Might be a DoS vector)

@mononaut
Copy link
Contributor Author

mononaut commented Mar 24, 2024

Added support for the optional maxfeerate parameter (via query string), and added some basic pre-checks to the POSTed raw transactions for valid hexadecimal-ness and string length.

maxfeerate: Option<f32>,
) -> Result<Vec<MempoolAcceptResult>> {
let params = match maxfeerate {
Some(rate) => json!([txhex, format!("{:.8}", rate)]),
Copy link
Member

Choose a reason for hiding this comment

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

f32::EPSILON (the smallest difference representable above 1.0) works out to about 11 sats when we consider 1.0 to be 1 BTC. So it can only really reliably show 7 decimal places.

Which I guess is fine if we consider 0.00001 BTC /kvB (5 decimal places) to be the minimum. (1 sat per vB) In which case showing 8 decimal places is pointless.

I wonder what the type is in Core. Might be worth it to use that, and round it to 8 if we use f64 and 7 if we use f32.

Copy link
Member

Choose a reason for hiding this comment

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

https://doc.rust-lang.org/std/primitive.f32.html#associatedconstant.EPSILON

0.00000011920929 is the smallest increment of f32.

Copy link
Member

Choose a reason for hiding this comment

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

https://doc.rust-lang.org/std/primitive.f64.html#associatedconstant.EPSILON

0.00000000000000022204460492503131 is smallest for f64

Copy link
Member

Choose a reason for hiding this comment

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

Or we could always just accept the sane sat/vB instead of BTC/kvB, and just slide the decimal place 5 to the left. lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testmempoolaccept in Core throws an error if you give it too many decimal places (e.g. from floating point rounding errors), so it needs to be truncated to something.

I picked 8.d.p out of habit, but yeah I'll check what Core's bounds actually are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

In that case you should use f64 so that 8 decimal points can be properly represented without truncating.

src/rest.rs Outdated Show resolved Hide resolved
src/rest.rs Outdated Show resolved Hide resolved
src/rest.rs Outdated Show resolved Hide resolved
@mononaut
Copy link
Contributor Author

  • Switched the input format to a JSON serialized list of raw hex strings
  • Enforced a maximum of 25 transactions per request (to match Core's limit)
  • Include the index of the transaction in the error response when per-tx precheck validation fails.

@mononaut
Copy link
Contributor Author

Switched maxfeerate param from f32 to f64

Copy link
Member

@softsimon softsimon left a comment

Choose a reason for hiding this comment

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

tested ACK @ [ac32e4b]

% curl -X POST -H "Content-Type: application/json" -d '["02000000000101a89d6d6e5475d9f17f153bd71c983170e6eb7d782966021ffea8e8bf03b791d50000000000fdffffff010d2a00000000000016001449b41a4c9fdd27da6d2c182078cbcd948b8e275a0247304402200d38e7b028f0f92778f3a7eca7208eee8a8fa397e7a1970eda63c5b6c0103e900220141239cc25d7b6e7ae6df833037d8d40b869d1d9ea57c0f71e516349eb9e0099012103de74b84d2a5715f317c23b84520a76a2b7ea5f7ba6302bd64ddf439eba546e11dd762700"]' http://localhost:3000/txs/test

[{"txid":"c27cdcc1b07096e8bc0b29c9868a4052231548c0b23f20d5437226baa42e004e","wtxid":"16586c10f96e52410c9a62ae80e5f8891b62ca887ff9722fa45653e355bdfc60","allowed":true,"vsize":110,"fees":{"base":0.00001562,"effective-feerate":0.000142,"effective-includes":["16586c10f96e52410c9a62ae80e5f8891b62ca887ff9722fa45653e355bdfc60"]},"reject-reason":null}]%

@softsimon softsimon merged commit eea193a into mempool Apr 15, 2024
7 checks passed
@softsimon softsimon deleted the mononaut/test-mempool-accept branch April 15, 2024 03:11
SatoKentaNayoro pushed a commit to boolnetwork/mempool-electrs that referenced this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants