search

namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
455 stars 147 forks source link

qt: added verification of minimal json in the configure name dialog box #537

Closed junekomeiji closed 3 months ago

junekomeiji commented 3 months ago

I added checking of JSON minimality in names/applications.cpp alongside the associated unit tests and added them to the configure name dialog box. The dialog box should tell you if you inputted an non-minimal JSON value and warn you against configuring your domain with said invalid value, but should allow you to continue anyway if you were to insist, as well as allow you to minimalise said JSON value before writing it onto the blockchain.

JeremyRand commented 3 months ago

Nice work Rose! Code review of 1d20b4759afdafd559a2d6cebaea3a03ac1824ee looks good, modulo the two tiny things I noted above. Will do a build/test now and report back.

JeremyRand commented 3 months ago

Manual GUI testing of 1d20b4759afdafd559a2d6cebaea3a03ac1824ee is flawless AFAICT. Running the unit tests now....

JeremyRand commented 3 months ago

Unit tests also pass on 1d20b4759afdafd559a2d6cebaea3a03ac1824ee.

I think this is mergeable once the above two trivial things are sorted out (the first one of which I'd like Daniel's opinion on before we decide on what to do).

JeremyRand commented 3 months ago

ACK 5d4163192db4a2d1ea62861a407ee33b7a208141. Unit tests pass, manual GUI testing works fine, code review looks good.

@domob1812 If this gets an ACK from you, is it OK if I merge to minimize delays?

JeremyRand commented 3 months ago

Code review of a6a991d62610c48fcbc8408f6bd394fb16b5d61f looks good to me, testing now....

JeremyRand commented 3 months ago

ACK a6a991d62610c48fcbc8408f6bd394fb16b5d61f; code review looks good, manual testing shows no issues, unit tests pass.

@domob1812 if this gets your ACK as well, is it OK if I merge it?