solana-program-library icon indicating copy to clipboard operation
solana-program-library copied to clipboard

Confidential transfers currently broken?

Open 0x0ece opened this issue 1 year ago • 5 comments

SPL currently depends on Solana SDK 1.17.6.

I think that this commit, included as of SDK 1.17.7, changes the generators of bulletproofs and therefore makes all range proofs invalid (confidential transfer, withdraw, confidential transfer with fee).

Specifically:

While:

  • Launch solana-test-validator v1.17.7+, e.g. current 1.17.16
  • Run the token transfers demo => doesn't work

The fix would be to upgrade the sdk version (or revert to the old generators).

0x0ece avatar Jan 17 '24 17:01 0x0ece

FYI, I was able to update up to 1.17.13 without compilation issues (see linked PR).

On 1.17.14 I'm getting:

error[E0004]: non-exhaustive patterns: `&UiExtension::GroupPointer(_)`, `&UiExtension::GroupMemberPointer(_)`, `&UiExtension::TokenGroup(_)` and 1 more not covered
   --> token/cli/src/output.rs:557:11
    |
557 |     match ui_extension {
    |           ^^^^^^^^^^^^ patterns `&UiExtension::GroupPointer(_)`, `&UiExtension::GroupMemberPointer(_)`, `&UiExtension::TokenGroup(_)` and 1 more not covered
    |
note: `UiExtension` defined here
   --> /home/ecesena/.cargo/registry/src/index.crates.io-6f17d22bba15001f/solana-account-decoder-1.17.14/src/parse_token_extension.rs:36:5
    |
15  | pub enum UiExtension {
    | --------------------
...
36  |     GroupPointer(UiGroupPointer),
    |     ^^^^^^^^^^^^ not covered
37  |     GroupMemberPointer(UiGroupMemberPointer),
    |     ^^^^^^^^^^^^^^^^^^ not covered
38  |     TokenGroup(UiTokenGroup),
    |     ^^^^^^^^^^ not covered
39  |     TokenGroupMember(UiTokenGroupMember),
    |     ^^^^^^^^^^^^^^^^ not covered
    = note: the matched value is of type `&UiExtension`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown, or multiple match arms
    |
916 ~         ),
917 ~         _ => todo!(),
    |

For more information about this error, try `rustc --explain E0004`.
error: could not compile `spl-token-cli` (lib) due to previous error

0x0ece avatar Jan 17 '24 18:01 0x0ece

@samkim-crypto is this expected?

Edit: thanks for the report and the fix! It's normal that 1.17.14+ has this issue, since there are new enum variants in that UiExtension type

joncinque avatar Jan 17 '24 20:01 joncinque

FYI, I created a zk-ledger using everything on 1.17.13:

  • solana-test-validator is 1.17.13
  • solana is 1.17.13
  • spl-token is from the PR, with all deps pointing to 1.17.13 => this works.

zk-ledger contains essentially the txs that were in the confidential transfer demo (deposit, confidential tf, withdraw).

Next, I upgraded my solana git repo to 1.17.16

  • if I run solana-ledger-tool 1.17.16 verify against the zk-ledger created with 1.17.13, it's successful (it was not successful against my old zk-ledger created with 1.17.6)

However, if I run solana-test-validator on 1.17.16

  • solana is either .13 or .16 (tested both)
  • spl-token is as above, linking .13 => unfortunately creating the zk-ledger doesn't work, which prob means there's something else happening between .13 and .16...

In summary:

  • tx created with .6 stop workin on .7+ (this issue)
  • tx created with .13 still work on .16 (good news, ie the PR fixes something)
  • upgrading spl-token to just .13 won't fix, because it doesn't seem to be working against .16 (bad news, i.e. the PR may not fix everything)

0x0ece avatar Jan 18 '24 00:01 0x0ece

In the process of exploration I also found this breaking change in v1.17.11. This is a non-issue wrt the PR/fix because we're skipping directly to 1.17.13.

I think the only way to avoid these issues across versions would be to include some tests with a binary blob. The current unit tests generate a ZK proof and verify it, thus they hide this type of cross-version incompatibilities.

For example this is a test that works on 1.17.13 and on 1.17.16:


    #[test]
    fn test_batched_range_proof_u128_firedancer_correctness() {
        let instruction_str = "09\
            d01237233159ce1d03b60584908f89191739fae2ccf2afab53a50f969f4eda12\
            e27e15681cca6af9c573ec822c7a4ff1037ef5a929ebd86cdb060170020a1601\
            c49560d2152ef4e657dd39190be3175b9d15a29ada9aa1d168d4fec53dc1a970\
            d475b1bc2801c967a64a099c2a5cc154780940235c78ac889544635314ceb254\
            0000000000000000000000000000000000000000000000000000000000000000\
            0000000000000000000000000000000000000000000000000000000000000000\
            0000000000000000000000000000000000000000000000000000000000000000\
            0000000000000000000000000000000000000000000000000000000000000000\
            4010201000000000\
            3ea7b3695740b168afa889c293f3173c160b3bcb2d895764f603642cf317790a\
            18165de4651dcfcca59f5fb798568eec30f6363c8bc52690cc62d5d59d73ae0f\
            d6d6f1799feac39f9f81daec8c17b9c6eaf628cf3b2bc5f5494a05962b8f3c6b\
            dcc2f9113e1e6828dbd7a06bd5904f0e2e36c7efb5e3a2d3e043856476bc2423\
            e7dcbebeca3f3e58ad8284a76c687e679ce6802dc3f7bd2420f15b517fd2b300\
            8aa04ede6f30a4d092eab9bd31ca65019f1abda573e561d0a0d9e3ef00204003\
            51abb1f943eb19d9462628591d826df0558d8b422d0cd10317ace85ca43a1d02\
            ce24d3bca0d5466e039571abb102b6637dc64f276f704de3d723e851a98f6a4e\
            7a668af3d1ec86f2831b4cf8bf56cefa842239613553414a5cb58c3986337042\
            56c803c3fb92a96769638a3adeb0755851f8af6b0c052e390ceecfe40ea36d07\
            0a7a547f7191ddc2dfa7388e0394e9799a03788996bc870bd564a9fd4978c432\
            26be25ac7b56feded658ad8eded6e29b7daf88ae175eaec43a4683eb9b4ea71e\
            5e6b04897ed3b8f328fa275e5db540e99f5a1198f78d4478191db52b307b8b42\
            ba13cab64c338035fb61aa01be9d19cd5fa3c336b08750372cb9b020af67b535\
            924a8f78f244d3bf695aafac8bc22e3f19edc9d20cb88a986dd997265c65b86a\
            306b6f2e3565413ce040342a84542fdcde368cd35f1b6d22b8af05474e149207\
            1cee6159c7a9cbf72d08e28cecee6d7d8a34a96e6636bce9aca0f03d6561cf16\
            ce68efd3a16c92e8a79c0645087d1b1df14c65c0d71fb6aa4bbb666f167c6b48\
            bca8ec4e34b5f70a4522609f4add869d997d3448a54693fa3174910698570227\
            b2ae1e51f280816b59655a62c3cab69d034860771d15a9f9ab6391bfb5e03d3a\
            928d8843df67de8b3c1aa96b26dc182facfe3cc580188f393035bc0dc47c1c0c\
            7b99a799c954aa0213cf804c8ae563d50a07d95053fca38bab6ce49502b36b0a\
            b96c923297a16e0b45cfb878c168cfa6dfcf22055c360427237d287109cdb90d";
        let instruction_data = hex::decode(instruction_str).unwrap();
        let proof_data = ProofInstruction::proof_data::<BatchedRangeProofU128Data, BatchedRangeProofContext>(instruction_data.as_slice()).unwrap();
        let res = proof_data.verify_proof();
        println!("{:?}", res);
        assert!(res.is_ok());
    }

0x0ece avatar Jan 18 '24 00:01 0x0ece

Yes, thanks for pointing this out and the fix! This is the expected behavior and the two PRs that you pointed out are the only ones that should affect confidential transfer proofs. The fact that spl depending on 17.13 does not seem to be working against 17.16 is a little odd and seems to require looking into for me. There is https://github.com/solana-labs/solana/pull/34765, which disables u256 range proof, though that shouldn't really affect confidential transfers without fee.

Adding the tests with binary blobs of proofs though seem like a good idea!

samkim-crypto avatar Jan 18 '24 08:01 samkim-crypto

Closing this as completed with https://github.com/anza-xyz/agave/pull/2814.

samkim-crypto avatar Sep 04 '24 09:09 samkim-crypto