James O'Beirne [ARCHIVE] on Nostr: 📅 Original date posted:2015-10-07 📝 Original message:This has been confirmed as ...
📅 Original date posted:2015-10-07
📝 Original message:This has been confirmed as a bug. Thanks again for reporting. I've filed a
fix here (https://github.com/bitcoin/bitcoin/pull/6777), and will be
writing tests to prevent regressions.
On Wed, Oct 7, 2015 at 4:32 PM, James O'Beirne <james.obeirne at gmail.com>
wrote:
> Hey, Daniel.
>
> Patch author here. Thanks for the diligence; I think this indeed may be an
> oversight, though I'm going to need to look into a bit more thoroughly at
> home. Curious that it didn't fail any of the automated tests.
>
> Correct me if I'm wrong, but the only actual invocation of that method is
> here
> <https://github.com/bitcoin/bitcoin/blob/master/src/rpcblockchain.cpp#L448>;
> (and even then, proxied through a few layers of CCoinView-machinery). In
> fact, this line
> <https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L48>; makes
> me suspect that the implementation of GetStats you reference may be dead
> code.
>
> In any case, you raise a good point: if users of CLevelDBWrapper go
> directly for the iterator, they run the risk of dealing with obfuscated
> data. This should be remedied somehow.
>
> I'll give it more look this evening.
>
> Thanks again for the find,
> James
>
> On Wed, Oct 7, 2015 at 10:25 AM, Daniel Kraft via bitcoin-dev <
> bitcoin-dev at lists.linuxfoundation.org> wrote:
>
>> Hi!
>>
>> I hope this is not a stupid question, but I thought I'd ask here first
>> instead of opening a Github ticket (in case I'm wrong).
>>
>> With the recently merged "obfuscation" patch, content of the
>> "chainstate" LevelDB is obfuscated by XOR'ing against a random "key".
>> This is handled by CLevelDBWrapper's Read/Write methods, which probably
>> cover most of the usecases.
>>
>> *However*, shouldn't it also be handled when iterating over the
>> database? In particular, I would expect that the obfuscation key is
>> applied before line 119 in txdb.cpp (i. e., while iterating over the
>> coin database in CCoinsViewDB::GetStats).
>>
>> Is there a reason why this need not be done there, or is this an actual
>> oversight?
>>
>> Yours,
>> Daniel
>>
>> --
>> http://www.domob.eu/
>> OpenPGP: 1142 850E 6DFF 65BA 63D6 88A8 B249 2AC4 A733 0737
>> Namecoin: id/domob -> https://nameid.org/?name=domob
>> --
>> Done: Arc-Bar-Cav-Hea-Kni-Ran-Rog-Sam-Tou-Val-Wiz
>> To go: Mon-Pri
>>
>>
>> _______________________________________________
>> bitcoin-dev mailing list
>> bitcoin-dev at lists.linuxfoundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20151007/ce7d4d3f/attachment-0001.html>;
📝 Original message:This has been confirmed as a bug. Thanks again for reporting. I've filed a
fix here (https://github.com/bitcoin/bitcoin/pull/6777), and will be
writing tests to prevent regressions.
On Wed, Oct 7, 2015 at 4:32 PM, James O'Beirne <james.obeirne at gmail.com>
wrote:
> Hey, Daniel.
>
> Patch author here. Thanks for the diligence; I think this indeed may be an
> oversight, though I'm going to need to look into a bit more thoroughly at
> home. Curious that it didn't fail any of the automated tests.
>
> Correct me if I'm wrong, but the only actual invocation of that method is
> here
> <https://github.com/bitcoin/bitcoin/blob/master/src/rpcblockchain.cpp#L448>;
> (and even then, proxied through a few layers of CCoinView-machinery). In
> fact, this line
> <https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L48>; makes
> me suspect that the implementation of GetStats you reference may be dead
> code.
>
> In any case, you raise a good point: if users of CLevelDBWrapper go
> directly for the iterator, they run the risk of dealing with obfuscated
> data. This should be remedied somehow.
>
> I'll give it more look this evening.
>
> Thanks again for the find,
> James
>
> On Wed, Oct 7, 2015 at 10:25 AM, Daniel Kraft via bitcoin-dev <
> bitcoin-dev at lists.linuxfoundation.org> wrote:
>
>> Hi!
>>
>> I hope this is not a stupid question, but I thought I'd ask here first
>> instead of opening a Github ticket (in case I'm wrong).
>>
>> With the recently merged "obfuscation" patch, content of the
>> "chainstate" LevelDB is obfuscated by XOR'ing against a random "key".
>> This is handled by CLevelDBWrapper's Read/Write methods, which probably
>> cover most of the usecases.
>>
>> *However*, shouldn't it also be handled when iterating over the
>> database? In particular, I would expect that the obfuscation key is
>> applied before line 119 in txdb.cpp (i. e., while iterating over the
>> coin database in CCoinsViewDB::GetStats).
>>
>> Is there a reason why this need not be done there, or is this an actual
>> oversight?
>>
>> Yours,
>> Daniel
>>
>> --
>> http://www.domob.eu/
>> OpenPGP: 1142 850E 6DFF 65BA 63D6 88A8 B249 2AC4 A733 0737
>> Namecoin: id/domob -> https://nameid.org/?name=domob
>> --
>> Done: Arc-Bar-Cav-Hea-Kni-Ran-Rog-Sam-Tou-Val-Wiz
>> To go: Mon-Pri
>>
>>
>> _______________________________________________
>> bitcoin-dev mailing list
>> bitcoin-dev at lists.linuxfoundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20151007/ce7d4d3f/attachment-0001.html>;