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.

Sign up for GitHub

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

Jump to bottom

Send OPENs to nulldata address #499

Closed
wants to merge 5 commits into from
Closed

Send OPENs to nulldata address #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b75f8a
address: don't restrict data length to 20 or 32
pinheadmz Aug 13, 2020
0301f47
wallet: do not remove wallet-name-map when disconnecting a block
pinheadmz Aug 14, 2020
4075ae8
wallet: send OPENs to unspendable nullData address
pinheadmz Aug 14, 2020
8edaea6
test: OPEN outputs no longer inserted as coins into txdb
pinheadmz Aug 14, 2020
c193247
pkg: update CHANGELOG
pinheadmz Aug 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ REGISTERing. If unpatched software was used to CLAIM a name already, that wallet
database is irreversibly corrupted and must be replaced.
See https://github.com/handshake-org/hsd/issues/454

- By default the wallet will now send OPEN covenants to an unspendable `nullData`
address (witness program version `31`, with minimum required data `0x0000`). This
will prevent 0-value coins from bloating the UTXO set in chainDB and walletDB.
It will also reduce the size of OPEN outputs in the blockchain by 18 bytes.
Applications that relied on the output address to detect when a user has sent an
OPEN must now rely on the input coins to an OPEN transaction for this purpose.

## v2.0.0

### Wallet API changes
Expand Down
2 changes: 0 additions & 2 deletions lib/primitives/address.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,6 @@ class Address extends bio.Struct {
throw new Error('Object is not an address.');

if (Buffer.isBuffer(data)) {
if (data.length !== 20 && data.length !== 32)
throw new Error('Object is not an address.');
return data;
}

Expand Down
1 change: 0 additions & 1 deletion lib/wallet/txdb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2155,7 +2155,6 @@ class TXDB {
ns.applyState(delta);

if (ns.isNull()) {
await this.removeNameMap(b, nameHash);
b.del(layout.A.encode(nameHash));
} else {
b.put(layout.A.encode(nameHash), ns.encode());
Expand Down
15 changes: 11 additions & 4 deletions lib/wallet/wallet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1091,9 +1091,6 @@ class Wallet extends EventEmitter {
async _importName(name) {
const nameHash = rules.hashName(name);

if (await this.txdb.hasNameState(nameHash))
throw new Error('Name already exists.');

const b = this.db.batch();
await this.wdb.addNameMap(b, nameHash, this.wid);
await b.write();
Expand Down Expand Up @@ -1561,7 +1558,17 @@ class Wallet extends EventEmitter {
if (start !== 0 && start !== height)
throw new Error('Name is already opening.');

const addr = await this.receiveAddress(acct);
// Send 0-value output to provably unspendable address:
// hs1lqqqqhuxwgy (mainnet) is the smallest valid nulldata address
// and represents witness version 31 folowed by data of 0x0000.
// Unspendable outputs do not bloat the UTXO set or the wallet's coin set.
const addr = Address.fromNulldata(Buffer.alloc(2));

// Since this address is not in our wallet, we must "manually" add the name
// to the wallet database's name map. Normally the wallet would do this in
// txdb.js connectNames() when a TX is inserted into the DB, and the wallet
// recognizes the output address.
await this.importName(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I missed it,

but does not this need to change rescan logic as well, so wallets can recover the OPENs they have sent?

Copy link
Member Author

Choose a reason for hiding this comment

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

importName() will add the name to the wallet's name map, which means the wallet will "track" all auction activity like bids etc even if the user did not bid themselves (this is weird, but its how the wallet works already). This will persist for rescans as well, especially because of this change: 0301f47

Since the wallet sent the open, it should recover the TX normally because its own keys were used for the inputs and change.

OPENs are weird things - they are just a marker for the chain and wallet (this is why they have 0 value), announcing to the network a namehash and its preimage. JJ told me OPENs weren't even added to the system until late in development because they didn't seem necessary, but definitely made wallet logic a lot easier.

We have a few rescan tests now in hsd, I'll rebase this PR and we can double check

Copy link
Contributor

Choose a reason for hiding this comment

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

sure,

I was wondering about importing wallet into fresh wallet instance.
I can test after rebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was wondering about importing wallet into fresh wallet instance.

Yes! this needs to be patched. Its' going to be in txdb.insert() or txdb.confirm() I think


const output = new Output();
output.address = addr;
Expand Down
15 changes: 4 additions & 11 deletions test/wallet-importname-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,8 @@ describe('Wallet Import Name', function() {
assert(ns4 === null);
});

it('should not re-import an existing name', async () => {
await assert.rejects(
alice.importName(name),
{message: 'Name already exists.'}
);
it('should ignore re-importing existing name', async () => {
await alice.importName(name);
});

it('should bid on names from Alice\'s wallet', async () => {
Expand Down Expand Up @@ -223,13 +220,9 @@ describe('Wallet Import Name', function() {
assert(!bid.own);
});

it('should not re-import name', async () => {
it('should ignore re-importing name', async () => {
await wclient.execute('selectwallet', ['charlie']);

await assert.rejects(
wclient.execute('importname', [name, 0]),
{message: 'Name already exists.'}
);
await wclient.execute('importname', [name, 0]);
});
});
});
22 changes: 11 additions & 11 deletions test/wallet-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1962,7 +1962,7 @@ describe('Wallet', function() {
// Check
let bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 2);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.coin, 1); // OPENs are sent to a null address
assert.strictEqual(bal.confirmed, 10e6);
assert.strictEqual(bal.unconfirmed, 10e6 - fee);
assert.strictEqual(bal.ulocked, 0);
Expand All @@ -1979,7 +1979,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 2);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.coin, 1);
assert.strictEqual(bal.confirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.ulocked, 0);
Expand All @@ -1995,7 +1995,7 @@ describe('Wallet', function() {
// Check
let bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 3);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.confirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.ulocked, lockup);
Expand All @@ -2012,7 +2012,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 3);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.confirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.ulocked, lockup);
Expand All @@ -2028,7 +2028,7 @@ describe('Wallet', function() {
// Check
let bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 4);
assert.strictEqual(bal.coin, 4);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.confirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (3 * fee));
assert.strictEqual(bal.ulocked, value);
Expand All @@ -2045,7 +2045,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 4);
assert.strictEqual(bal.coin, 4);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.confirmed, 10e6 - (3 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (3 * fee));
assert.strictEqual(bal.ulocked, value);
Expand Down Expand Up @@ -2126,7 +2126,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 2);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.coin, 1);
assert.strictEqual(bal.confirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.ulocked, 0);
Expand All @@ -2142,7 +2142,7 @@ describe('Wallet', function() {
// Check
let bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 2);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.coin, 1);
assert.strictEqual(bal.confirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (1 * fee));
assert.strictEqual(bal.ulocked, 0);
Expand All @@ -2159,7 +2159,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 3);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.confirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.ulocked, lockup);
Expand All @@ -2175,7 +2175,7 @@ describe('Wallet', function() {
// Check
let bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 3);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.coin, 2);
assert.strictEqual(bal.confirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (2 * fee));
assert.strictEqual(bal.ulocked, lockup);
Expand All @@ -2192,7 +2192,7 @@ describe('Wallet', function() {
// Check
bal = await wallet.getBalance();
assert.strictEqual(bal.tx, 4);
assert.strictEqual(bal.coin, 4);
assert.strictEqual(bal.coin, 3);
assert.strictEqual(bal.confirmed, 10e6 - (3 * fee));
assert.strictEqual(bal.unconfirmed, 10e6 - (3 * fee));
assert.strictEqual(bal.ulocked, value);
Expand Down