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

Allow multiple updates of a name per block #322

Merged
merged 3 commits into from
Jul 9, 2019
Merged

Allow multiple updates of a name per block #322

merged 3 commits into from
Jul 9, 2019

Conversation

domob1812
Copy link

@domob1812 domob1812 commented Jun 30, 2019 •
edited
Loading

This removes the mempool-restriction of allowing only one update per name and block. That restriction was introduced originally to simplify the mempool implementation, but is actually easy to lift.

With this change, we allow a chain of updates of one name to be in the mempool at once; this can be useful e.g. to quickly fix an erraneous previous update without the need to wait for confirmations in between. For name_new and name_firstupdate, no changes are made. Only updates are allowed to be chained on a pending operation, although updates can be chained to a name_firstupdate.

Note that the consensus layer of Namecoin already allows such chains, and the restriction lifted is purely a policy one (in other words, this is not a fork of any sort). For now, we only remove the restriction from the mempool itself; the name_update RPC method still won't allow creating such chains. We will remove the RPC restriction in a second step later on, once miners and nodes have updated to the new policy so that such chains of updates can actually be expected to be relayed and mined.

This mostly resolves #304, except that we will have to remove the RPC restriction at a later point in time as well.

@domob1812
Copy link
Author

@JeremyRand and others, please take a look at this when convenient. I will myself do some more tests, e.g. that a node relaying those transactions won't get banned on mainnet for it. If there are no objections within a week, I will merge this.

@domob1812
Copy link
Author

I did the following test on mainnet: I connected a node built form this PR + removed check in name_update to another mainnet node I run on a VPS. Then I updated the name kraft twice from that node, first to reserved 1 and then to reserved 2. This gave me both transactions in the local node's mempool, and only the first transaction in the remote (unupdated) node's. I verified that this did not ban or increase the banscore of the modified node.

After a couple of blocks, both transactions were mined just fine. The first immediately, and the second after a rebroadcast of the transaction after the first was mined. So all seems fine.

@domob1812
Remove CNameMemPool::getTxForName.
99e1568 
The function getTxForName is only used for name_pending filtered by
name.  There, we can instead just go through the full mempool anyway
and then filter the name scripts.

With namecoin#304 coming up,
there will no longer be a single transaction per name.  Thus by removing
the unnecessary function now, we avoid having to complicate it with
returning multiple names.
@domob1812
Allow multiple updates of a name per block.
edcfc22 
This removes the mempool-restriction of allowing only one update
per name and block.  That restriction was introduced originally to
simplify the mempool implementation, but is actually easy to lift.

With this change, we allow a chain of updates of one name to be in the
mempool at once; this can be useful e.g. to quickly fix an erraneous
previous update without the need to wait for confirmations in between.
For name_new and name_firstupdate, no changes are made.  Only updates
are allowed to be chained on a pending operation, although updates can
be chained to a name_firstupdate.

Note that the consensus layer of Namecoin already allows such chains,
and the restriction lifted is purely a policy one (in other words, this
is not a fork of any sort).  For now, we only remove the restriction
from the mempool itself; the name_update RPC method still won't allow
creating such chains.  We will remove the RPC restriction in a second
step later on, once miners and nodes have updated to the new policy
so that such chains of updates can actually be expected to be relayed
and mined.

This fixes namecoin#304.
@domob1812
Copy link
Author

Rebased the patch to the latest upstream changes. I will merge that tomorrow, and will then also backport it to the 0.18 branch - we should try to get miners and relaying nodes updated as soon as possible (although not necessarily ask them to do it, as it is not "required" like a fork).

@domob1812
Add release notes entry about multi updates.
78a8836 
Describe the changes done to the mempool policy (allowing multiple name
updates) for the 0.19 release notes.
@domob1812
Copy link
Author

Added release notes in 78a8836, will merge this now.

@domob1812 domob1812 merged commit 78a8836 into namecoin:master Jul 9, 2019
domob1812 added a commit that referenced this pull request Jul 9, 2019
@domob1812
Merge #322: Allow multiple updates of a name per block
7b089bf 
78a8836 Add release notes entry about multi updates. (Daniel Kraft)
edcfc22 Allow multiple updates of a name per block. (Daniel Kraft)
99e1568 Remove CNameMemPool::getTxForName. (Daniel Kraft)

Pull request description:

  This removes the mempool-restriction of allowing only one update per name and block.  That restriction was introduced originally to simplify the mempool implementation, but is actually easy to lift.

  With this change, we allow a chain of updates of one name to be in the mempool at once; this can be useful e.g. to quickly fix an erraneous previous update without the need to wait for confirmations in between.  For `name_new` and `name_firstupdate`, no changes are made.  Only updates are allowed to be chained on a pending operation, although updates can be chained to a `name_firstupdate`.

  Note that the consensus layer of Namecoin already allows such chains, and the restriction lifted is purely a policy one (in other words, this is not a fork of any sort).  For now, we only remove the restriction from the mempool itself; the `name_update` RPC method still won't allow creating such chains.  We will remove the RPC restriction in a second step later on, once miners and nodes have updated to the new policy so that such chains of updates can actually be expected to be relayed and mined.

  This mostly resolves #304, except that we will have to remove the RPC restriction at a later point in time as well.

Top commit has no ACKs.

Tree-SHA512: 8515a179449555ee62faf9b0f9b4ba98da2f541c95181876ee6095c62077bf4f7ae09c1265f760c39b723e513f0367b262b1b55724b4c2d74e89825eae513a63
@domob1812 domob1812 deleted the more-updates branch July 9, 2019 07:18
@domob1812 domob1812 added this to the nc0.19 milestone Jul 9, 2019
domob1812 added a commit to xaya/xaya that referenced this pull request Jul 9, 2019
@domob1812
Merge branch 'namecoin'
7e31ae3 
Rework namecoin/namecoin-core#322 for the
changes between Xaya and Namecoin.  In particular, the absence of
name expiration and name_register instead of new / firstupdate.
domob1812 added a commit to xaya/xaya that referenced this pull request Jul 9, 2019
@domob1812
Split name mempool code into separate file.
1f479f5 
This is a backport of 1d9535c, which
itself is basically namecoin/namecoin-core#321
for the 1.2 release branch.

We need this as a first step for backporting
namecoin/namecoin-core#322 to Xaya 1.2.
Unlike Namecoin, having this functionality in Xaya is quite important
and useful, thus we should make sure it gets included in a release as
soon as possible.
domob1812 added a commit to xaya/xaya that referenced this pull request Jul 9, 2019
@domob1812
Split name mempool code into separate file.
60ba8d6 
This is a backport of 1d9535c, which
itself is basically namecoin/namecoin-core#321
for the 1.2 release branch.

We need this as a first step for backporting
namecoin/namecoin-core#322 to Xaya 1.2.
Unlike Namecoin, having this functionality in Xaya is quite important
and useful, thus we should make sure it gets included in a release as
soon as possible.
domob1812 added a commit to xaya/xaya that referenced this pull request Jul 9, 2019
@domob1812
Allow multiple name updates per block.
f5e97b8 
This is a backport of 7e31ae3, which
essentially is namecoin/namecoin-core#322,
for the Xaya 1.2 release branch.

This allows a name to be updated multiple times in the mempool (on the
consensus level, this is already allowed in blocks).  The name_update
RPC method still does not allow this feature, as we need miners and
relaying nodes to update first.

By backporting this, we can make sure that miners and relaying nodes
update as soon as possible, so that we can soon take advantage of this
feature in actual games.
@domob1812 domob1812 mentioned this pull request Jul 9, 2019
Merged
domob1812 added a commit to xaya/xaya that referenced this pull request Jul 9, 2019
@domob1812
Merge #96: Allow multiple name updates in the mempool
598c929 
f5e97b8 Allow multiple name updates per block. (Daniel Kraft)
60ba8d6 Split name mempool code into separate file. (Daniel Kraft)

Pull request description:

  With this change, the mempool allows multiple updates (in a chain) of one name.  This is already allowed in blocks, so not a consensus change.  `name_update` (the RPC command) still prevents such updates to be created, which is a restriction we can lift once sufficient miners and relaying nodes have updated.

  This is a backport of namecoin/namecoin-core#322 to the 1.2 release branch, so that we can get miners and nodes updated as soon as possible.  It has been merged already to the master branch through Namecoin (where it won't make it into the 0.18 branch).

Tree-SHA512: c6898a1b91972426a24758312c0c3793ea498aa2ec1b60a59ae4e75735f5bccd8001302a34b9579c421ebc365d133fb1760f060dce2df7fcb0dd28cf0ceb438a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement mempool
Projects
None yet
Milestone
nc0.19
Development

Successfully merging this pull request may close these issues.

Allow multiple updates of one name in one block
1 participant
@domob1812