Creating too many BIDs makes the REVEAL exceed TX policy weight

[turbomaze]

Currently, the wallet exposes one RPC call to send/create reveals, and it attempts to reveal all of your wallet's bids for a name. It's possible to have so many bids that the resulting transaction exceeds the TX policy weight, which prevents it from getting mined.

It's easier to make this happen when your coins are very fragmented, but I can consistently encounter this behavior when I call sendreveal on a domain with ~595 bids. Note: everything here likely also applies to redeems/etc, and also to the create* version of those calls.

Solutions

I'm happy to implement the workaround for this, but there are a couple of options here and I would appreciate some help thinking through which is best:

Option 1: Limit # reveals with a count

Accept a "count" parameter that will reveal up to ${count} bids in the final transaction. This would require the client to repeatedly call sendreveal to ensure all of their bids get revealed.

This is the easiest to implement and also easiest to understand.

Option 2: Reveal as many as possible

Always reveal the maximal number of bids. It's the responsibility of the client to call sendreveal again if they want to reveal the rest of their bids. This improves on the default behavior which will forever fail to generate a viable transaction.

This is harder to implement because the relationship between # bids revealed and TX size is nonlinear -- depending on the lockup amount, the number of inputs required to pay for each additional bid can vary greatly. Would need to do some weird (& potentially expensive) binary search to determine how many bids can fit in a TX.

Option 3: Generate multiple transactions

Implement option 2, but perform that logic repeatedly until all of the bids have been revealed. This results in multiple transactions being created at the same time.

This results in the clearest behavior for the user, since they just need to worry about what they want to get done ("reveal all my bids please") and not how it will happen ("reveal these 50, then these 70, etc etc"). Shares similar shortcomings to option 2, but has a slightly stranger spec since it involves broadcasting multiple TXs.

--

I'm leaning towards option 1 since on mainnet, the reveal period is 10 days, so it's not a big deal if clients need to remember to manually submit multiple TXs during that time frame.

Other considerations: if option 2/3, then this should probably be a new RPC call (sendlotsofreveals etc), but if option 1, then I think it would make sense to add an additional optional parameter with a large default value to the normal sendreveal call.

[tynes]

Regarding this:

It's possible to have so many bids that the resulting transaction exceeds the TX policy weight, which prevents it from getting mined.

Are you sure that its the TX policy weight? That would just mean that its not gossipped between full node mempools but it still could be valid in a block.

hsd/lib/primitives/tx.js

Lines 1125 to 1130 in f9d00ce

	checkStandard() {
	if (this.version > policy.MAX_TX_VERSION)
	return [false, 'version', 0];
	if (this.getWeight() > policy.MAX_TX_WEIGHT)
	return [false, 'tx-size', 0];

Technically these types of transactions can still be included in blocks, up to the consensus maximum transaction size. They could be given to miners out of band using personal relationships but I'm not sure of the impacts on the network of that.

hsd/lib/primitives/tx.js

Lines 1015 to 1023 in f9d00ce

	checkSanity() {
	if (this.inputs.length === 0)
	return [false, 'bad-txns-vin-empty', 100];
	if (this.outputs.length === 0)
	return [false, 'bad-txns-vout-empty', 100];
	if (this.getBaseSize() > consensus.MAX_TX_SIZE)
	return [false, 'bad-txns-oversize', 100];

---

Great solutions, listing thoughts on each below.

Option 1

+ Easy to implement
- Opaque around which names

Option 2

- Seems more complex
- Opaque around which names

Option 3

+ Medium seeming complexity
+ Good UX
- Opaque around names
- Privacy implications

If all transactions created at the same time are broadcast together, it will link keys in the same HD tree together.

Option 4

Client sends query param that is a list of names, the call fails if the exact names cannot be included. It seems like it would expose a nice way to finely control which names to send along with the number of outputs (1 name per output), which is a good proxy for size. (edit: number of inputs could vary a bit, hmm) If there isn't an easy way to get the names owned by a wallet/account then it might not be ideal.

---

Out of all of the options, I like options 3 and 4. What do you think? What could you see working well?

[turbomaze]

Are you sure that its the TX policy weight? That would just mean that its not gossipped between full node mempools but it still could be valid in a block.

Technically true but most people will not have the ability to mine blocks during mainnet-level hashpower so practically, if it's not gossiped it's not getting mined.

Notes re: 4
I haven't actually tested revealing multiple names in one of these calls. I recall one of the RPC endpoints revealing/redeeming all names if no name was supplied, but I have been encountering this issue when it's just 1 name with a lot of bids. Seems like option 4 can be implemented via multiple calls of option 1 (one call per name). 4 makes the binary search problem harder since the user would need to search across multiple dimensions (each name included is its own dimension).

After thinking a little more I don't like solutions that require the client to think about how they plan to get all of their bids revealed. I think it'd be best if the API were declarative (something like option 3).

My main worry is it's going to be difficult to figure out how many bids to include in each transaction. I don't yet know how expensive it is to generate 10 different transactions (i.e. a binary search of ~1000 bids), but generating just one transaction with 600 reveals has been quite slow. With that being said, that binary searching work either needs to be performed by the wallet after 1 call or manually by the client across 10 calls (seeing which fail/succeed), so it doesn't seem we can save any cycles there.

[pinheadmz]

sendreveal already takes a name argument doesn't it? If left empty, the wallet will try to reveal all. So in your specific case, you could manually send multiple reveal TXs.

In the case where even a single name has too many bids, I like option 1 but the limit should be policy, not a parameter. A new RPC sendrevealbatch or single boolean argument would take care of the rest.

Example: as the wallet adds "all" reveals to the mtx, it throws when the policy size limit is reached (estimated). The error message informs the user to use sendrevealbatch which will BROADCAST the tx when the limit is reached. The user will have to run the command again until the response is "nothing to reveal".

[turbomaze]

sendreveal already takes a name argument doesn't it? If left empty, the wallet will try to reveal all. So in your specific case, you could manually send multiple reveal TXs.

To clarify: I've never actually used sendreveal without providing a name. The errors I'm running into are for revealing a single name.

In the case where even a single name has too many bids, I like option 1 but the limit should be policy, not a parameter. A new RPC sendrevealbatch or single boolean argument would take care of the rest.
Example: as the wallet adds "all" reveals to the mtx, it throws when the policy size limit is reached (estimated). The error message informs the user to use sendrevealbatch which will BROADCAST the tx when the limit is reached. The user will have to run the command again until the response is "nothing to reveal".

I like this solution, I think it's actually closest to option 2. I haven't dug into the code that generates these TXs yet, so it's good to hear that we can monitor the mtx size as we add reveal TXOs. I wrote these options for the worst-case scenario where generating the mtx is a black box. If we can halt the second we encounter the bid that breaks the mtx-camel's back, then option 2 is easy to implement.

One note: not in love with the UX of calling sendrevealbatch until hitting a particular error message. I wouldn't want to incur the additional latency that comes with a noop sendrevealbatch call that reports "nothing to reveal" each time I reveal some bids. Would be cleaner I think to return a boolean in addition to the transaction that encodes whether or not there are more bids to reveal.

^-- I can start implementing this call on Monday unless anyone has any more comments

[pinheadmz]

monitor the mtx size

Well, it'll prob be easiest to just set a hard coded value like MAX_REVEALS_PER_TX, and then just add a counter right here in wallet.makeReveal() inside the for loop:

hsd/lib/wallet/wallet.js

Lines 1778 to 1782 in e3be17c

	const bids = await this.getBids(nameHash);
	const mtx = new MTX();
	for (const {prevout, own} of bids) {

[pinheadmz]

And I hear ya about the noop, etc - and for Namebase you're going to be doing this all automatically anyway. So, maybe make a call in your application layer that gets the number of bids you need to reveal ahead of time, then (given MAX_REVEALS_PER_TX) you'll know how many times to call sendreveal...

I think as far as TX size for that limit, we can assume the number of bids you have will be equal to the number of inputs and outputs, plus one each (extra funds to pay the miner fee, and change address). So it should be relatively easy to figure out where the limit should be.

[turbomaze]

Good point about 1 input/output each -- I suppose we can just do the math for the worst-case size of bid (assume they're all p2sh addresses?). Fees are a very tricky since these transactions get quite large. Depending on the current fee market, and coin fragmentation, I could easily imagine this large reveal call requiring multiple inputs to pay for the fee (still just 1 for change though).

I'm not sure how to compute the size of the inputs that pay the fee. Remind me, the redeem script for each input contributes to the tx weight, right? Depending on the multisig setup for the specific coins paying for the outputs, the script may include a bunch of conditional statements with OP_TYPE calls etc for all the covenants, multiple pubkeys, etc.

I'm hesitant of any heuristic-based (i.e. MAX_REVEALS_PER_TX) size estimates because if our estimate of the maximum weight contributed by the inputs is insufficiently conservative, then the outcome is not good. It would no longer be possible to interact with the wallet's exposed interface and reveal any of the bids for that name (ignoring manually building and signing the tx). The user would need to modify wallet code to get any of their reveals broadcasted.

I would want this new RPC call to guarantee that >0 bids will be revealed as a result of the call. Then it's at least possible to slowly chip away and make progress. Maybe the RPC call I'm looking for isn't "sendrevealbatch", but there should be something that guarantees progress for anomalous cases. We could definitely do some envelope math and find a sufficiently conservative MAX_REVEALS_PER_TX number, but any number that "guarantees" prorgess would probably be drastically too conservative for ordinary usage.

[tynes]

Hey @turbomaze, any new thoughts on this?

To clarify: I've never actually used sendreveal without providing a name. The errors I'm running into are for revealing a single name.

Thank you for clarifying, it didn't really occur to me that revealing for a single name would run into this issue at first, but at a larger scale that makes sense.

It does sound like you would want some new code for efficiency purposes. Not sure if it would be useful to split the users up into more accounts (see code snippet below), this would be wasteful from a batching perspective but would preserve privacy on the network a bit more. In reality, we shouldn't expect people to preserve privacy at the expense of saving money, the two should be aligned. This also still doesn't really solve the problem, the problem could still appear if a single account has many reveals.

hsd/lib/wallet/rpc.js

Line 2046 in e1c0281

const tx = await wallet.sendReveal(opts.name, { account: opts.account });

Have you considered the wallet HTTP API? The wallet RPC API cannot be safely consumed by multiple threads/processes if more than 1 wallet is in use, due to their being a global wallet object. See the RPC selectwallet:

hsd/lib/wallet/rpc.js

Line 1696 in e1c0281

this.wallet = wallet;