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

Reject claiming of revoked name immediately #4331

Open
davidyuk opened this issue Apr 15, 2024 · 7 comments
Open

Reject claiming of revoked name immediately #4331

davidyuk opened this issue Apr 15, 2024 · 7 comments
Labels
need/triage New issues which need to be categorized

Comments

@davidyuk
Copy link
Member

davidyuk commented Apr 15, 2024

If I'm trying to claim the revoked name, node firstly accepts transaction, but after that it disappears from the mempool. Would be good to reject NameClaim transaction immediately. Basically, it is the same case as #4310.

Reproduction

import { Node, AeSdk, MemoryAccount } from '@aeternity/aepp-sdk'; // 13.3.0
const node = new Node('https://testnet.aeternity.io');
const aeSdk = new AeSdk({
  nodes: [{ name: 'testnet', instance: node }],
  accounts: [
    new MemoryAccount('9ebd7beda0c79af72a42ece3821a56eff16359b6df376cf049aee995565f022f840c974b97164776454ba119d84edc4d6058a8dec92b6edc578ab2d30b4c4200'),
  ],
});
const name = await aeSdk.aensPreclaim('revoked-revoked.chain');
console.log('claim', await name.claim());

It fails with

RestError: v3/transactions/th_ZA8xtc9ZWuPjk9Gs39hBX1kVbzh62TAR6Fqjr4E6NTMVGhh5R error: Transaction not found

When NameClaimTx gets finally removed from mempool.

@davidyuk davidyuk added the need/triage New issues which need to be categorized label Apr 15, 2024
@hanssv
Copy link
Member

hanssv commented Apr 15, 2024

Isn't this one different, though? A revoked name is only blocked for a period of time, right? While a non-payable account is never going to become payable.

Still, it could be added, but it would need a bit of logic comparing the TX TTL to the revoke-height, etc. Would be a bit finicky to test 🤔

@davidyuk
Copy link
Member Author

davidyuk commented Apr 23, 2024

A revoked name is only blocked for a period of time

I was thinking that it is forever, but protocol says it is revoked only for 4 days.

comparing the TX TTL to the revoke-height

Won't be more accurate compare a minimum between TX TTL and max amount of blocks a TX can stay in mempool?

@hanssv
Copy link
Member

hanssv commented May 2, 2024

You also have the case where there is a PreClaim involved, that is only valid for 300 generations (I think??) - so lot's of cases to cover in testing... Is it worth it?
cc @happi @dincho @ThomasArts

@dincho
Copy link
Member

dincho commented May 23, 2024

My general opinion is that the node should not accept a TX if (is known to be) would be then dropped immediately from the pool. I'm not very familiar with the NS protocol to comment on the specific issue.

@ThomasArts
Copy link
Contributor

I don't think this should be solved on the node level.
If you claim a revoked name, you must have a reason to do so. For example, you may want to have that name. If you then claim it before the revoke period is fully completed, just to be sure to be the first with that claim, then posting it a bit earlier and have it in the mempool until you get allowed would make sense.

I think a reasonable wallet could warn a user that the name they try to get has been revoked and won't be available until .... The user is then free to choose whether to post it or not. If the transaction gets rejected and disappears from the pool, then the wallet may have a strategy to resend.

Otherwise, we are making nodes more and more complex to test, get all kind of exceptional logic for all kind of corner cases optimizations. I would dislike that.

@dincho
Copy link
Member

dincho commented May 23, 2024

I agree in general with @ThomasArts as well, however the logic for that validation is already there, but runs on mempool level

@davidyuk
Copy link
Member Author

then posting it a bit earlier and have it in the mempool until you get allowed would make sense

To address this case, the node may compare tx's ttl with a block at which the name would be available. And still, reject a tx if the ttl is too small or if ttl is 0 but tx would be removed from mempool by timeout.

For historical reasons, sdk tries to be developer-friendly by validating transactions before submission. A stuck tx in mempool without an error message is a poor developer experience as I see. If the node won't have a such check then it looks consistent to implement it on the sdk side, but it is expensive because it requires an extra http request.

Actually, the node doesn't expose a block height at which the name would be available

// http://localhost:3013/v3/names/cQ6QywWtsqEyo8AwktmdK8DShhkRBH.chain
{
  "reason": "Name revoked",
  "error_code": "name_revoked"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage New issues which need to be categorized
Projects
None yet
Development

No branches or pull requests

4 participants