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

Schemas use msat where parameter is actually sat #7333

Closed
rustyrussell opened this issue May 24, 2024 · 2 comments
Closed

Schemas use msat where parameter is actually sat #7333

rustyrussell opened this issue May 24, 2024 · 2 comments
Assignees
Labels
Milestone

Comments

@rustyrussell
Copy link
Contributor

Audit needed, and GRPC probably needs fixing! Otherwise GRPC will be out by 1000, which is awful (unless it uses the msat suffix, which it probably should?!)

@daywalker90 @ShahanaFarooqui

@daywalker90
Copy link
Contributor

I don't understand the issue besides it being a bit weird to pass msat values to something that touches onchain bitcoin, but this test passes:

import logging
import os

import grpc
import pytest
from pyln import grpc as clnpb
from pyln.testing.fixtures import *  # noqa: F403

LOGGER = logging.getLogger(__name__)


def test_grpc_amount(node_factory, bitcoind):  # noqa: F811
    LOGGER = logging.getLogger(__name__)
    port = node_factory.get_unused_port()
    l1, l2 = node_factory.get_nodes(
        2,
        opts=[
            {"log-level": "io", "grpc-port": str(port)},
            {"log-level": "info"},
        ],
    )
    l1.fundwallet(10_000_000)

    l2info = l1.rpc.getinfo()

    CLN_DIR = l2info["lightning-dir"]
    LOGGER.info(l2info["lightning-dir"])

    with open(os.path.join(CLN_DIR, "client.pem"), "rb") as f:
        client_cert = f.read()
    with open(os.path.join(CLN_DIR, "client-key.pem"), "rb") as f:
        client_key = f.read()

    # Load the server's certificate
    with open(os.path.join(CLN_DIR, "server.pem"), "rb") as f:
        server_cert = f.read()

    CLN_GRPC_HOLD_HOST = f"localhost:{port}"

    os.environ["GRPC_SSL_CIPHER_SUITES"] = "HIGH+ECDSA"

    # Create the SSL credentials object
    creds = grpc.ssl_channel_credentials(
        root_certificates=server_cert,
        private_key=client_key,
        certificate_chain=client_cert,
    )
    # Create the gRPC channel using the SSL credentials
    channel = grpc.secure_channel(
        CLN_GRPC_HOLD_HOST,
        creds,
        options=(("grpc.ssl_target_name_override", "cln"),),
    )

    # Create the gRPC stub
    node_stub = clnpb.NodeStub(channel)
    l1.rpc.connect(l2.info["id"] + "@localhost:" + str(l2.port))
    request = clnpb.FundchannelRequest(
        amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=1_000_000_500)),
        id=bytes.fromhex(l2.info["id"]),
    )
    with pytest.raises(Exception, match="should be a satoshi amount"):
        node_stub.FundChannel(request)
    
    request2 = clnpb.FundchannelRequest(
        amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=1_000_000_000)),
        id=bytes.fromhex(l2.info["id"]),
    )
    node_stub.FundChannel(request2)
    check_chan = l1.rpc.listpeerchannels()["channels"][0]
    assert check_chan["total_msat"] == 1_000_000_000

From io log-level:

{"jsonrpc":"2.0","id":"1/cln:fundchannel#48/spenderp:multifundchannel#1","method":"multifundchannel","params":{"destinations":[{"id":"022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59","amount":"1000000000msat"}]}}

I don't know how to break grpc here so that it would pass an amount that is off by x1000

@endothermicdev
Copy link
Collaborator

Closing as resolved with #7336. I was not able to find any other lingering schema mismatches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants