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

save some space in rct transactions #5052

Merged
merged 5 commits into from Jan 28, 2019

Conversation

Projects
None yet
5 participants
@moneromooo-monero
Copy link
Contributor

moneromooo-monero commented Jan 8, 2019

No description provided.

@moneromooo-monero moneromooo-monero changed the title Rctb save some space in rct transactions Jan 8, 2019

@@ -79,9 +79,12 @@ namespace
}

namespace rct {
Bulletproof proveRangeBulletproof(keyV &C, keyV &masks, const std::vector<uint64_t> &amounts)
Bulletproof proveRangeBulletproof(keyV &C, keyV &masks, const std::vector<uint64_t> &amounts, epee::span<const key> sk)

This comment has been minimized.

Copy link
@stoffu

stoffu Jan 10, 2019

Contributor

const ref?

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jan 10, 2019

Author Contributor

vtnerd complains about const refs for those, as they're small and apparently the code generated for by-value is slightly better than for const ref. So I'm starting to do this when I remember.

@@ -2449,6 +2449,18 @@ bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context
}
}

// from v10, allow bulletproofs v2
if (hf_version < 10) {

This comment has been minimized.

Copy link
@stoffu

stoffu Jan 10, 2019

Contributor

Why not use HF_VERSION_SMALLER_BP?

virtual bool ecdhEncode(rct::ecdhTuple & unmasked, const rct::key & sharedSec) = 0;
virtual bool ecdhDecode(rct::ecdhTuple & masked, const rct::key & sharedSec) = 0;
virtual bool ecdhEncode(rct::ecdhTuple & unmasked, const rct::key & sharedSec, bool short_amount) = 0;
virtual bool ecdhDecode(rct::ecdhTuple & masked, const rct::key & sharedSec, bool short_amount) = 0;

This comment has been minimized.

Copy link
@stoffu

stoffu Jan 10, 2019

Contributor

Maybe rename these bool short_amount to bool v2 as done in rctOps.cpp?

@@ -130,18 +130,3 @@ TEST(device, ecdh32)
ASSERT_EQ(tuple2.senderPk, tuple.senderPk);
}

TEST(device, ecdh8)

This comment has been minimized.

Copy link
@stoffu

stoffu Jan 10, 2019

Contributor

Why remove this test?

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jan 10, 2019

Author Contributor

The encode is not really an encode since the first input is now ignored as it's deterministic, so the output will not equal the input anymore after a encode/decode run.

@@ -831,27 +831,6 @@ TEST(ringct, HPow2)

static const xmr_amount test_amounts[]={0, 1, 2, 3, 4, 5, 10000, 10000000000000000000ull, 10203040506070809000ull, 123456789123456789};

TEST(ringct, ecdh_roundtrip)

This comment has been minimized.

Copy link
@stoffu

stoffu Jan 10, 2019

Contributor

Why remove this test?

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jan 10, 2019

Author Contributor

Same reason as above.

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:rctb branch from 2399597 to 5a06a4d Jan 10, 2019

@SarangNoether

This comment has been minimized.

Copy link
Contributor

SarangNoether commented Jan 22, 2019

The encode/decode math checks out.

moneromooo-monero added some commits Jan 6, 2019

add a bulletproof version, new bulletproof type, and rct config
This makes it easier to modify the bulletproof format
ringct: the commitment mask is now deterministic
saves space in the tx and is safe

Found by knaccc
ringct: remove unused senderPk from ecdhTuple
This was an early ringct field, which was never used in production

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:rctb branch from 5a06a4d to b6534c4 Jan 22, 2019

@moneromooo-monero

This comment has been minimized.

Copy link
Contributor Author

moneromooo-monero commented Jan 23, 2019

I changed genCommitmentMask to be closer to ecdhHash at sarang's request. No functional change.

@b-g-goodell

This comment has been minimized.

Copy link

b-g-goodell commented Jan 23, 2019

The encode/decode math checks out.

Agreed with sarang. ecdhHash, genCommitmentMask, xor8, ecdhEncode, ecdhDecode look correct to me.

@fluffypony
Copy link
Collaborator

fluffypony left a comment

Reviewed

@fluffypony fluffypony merged commit b6534c4 into monero-project:master Jan 28, 2019

6 of 10 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-static-win64 Build done.
Details
buildbot/monero-linux-armv7 Build done.
Details
buildbot/monero-linux-armv8 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-osx-10.13 Build done.
Details
buildbot/monero-static-ubuntu-amd64 Build done.
Details

fluffypony added a commit that referenced this pull request Jan 28, 2019

Merge pull request #5052
b6534c4 ringct: remove unused senderPk from ecdhTuple (moneromooo-monero)
7d37598 ringct: the commitment mask is now deterministic (moneromooo-monero)
99d946e ringct: encode 8 byte amount, saving 24 bytes per output (moneromooo-monero)
cdc3cce ringct: save 3 bytes on bulletproof size (moneromooo-monero)
f931e16 add a bulletproof version, new bulletproof type, and rct config (moneromooo-monero)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.